Richard W.M. Jones
2018-Oct-01 11:28 UTC
[Libguestfs] [PATCH API PROPOSAL 0/2] inspection: Add network interfaces to inspection data.
As part of the fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1626503 I'm proposing to add two new APIs to fetch information about the list of network interfaces of an existing guest. These two patches outline the proposed API but with no implementation or tests. However they can be applied and compiled. Please see the second patch for the proposed API. I have a mostly working implementation for Red Hat-style ifcfg-* files, and both Debian /etc/network/interfaces and Windows registry look do-able using the same architecture. Rich.
Richard W.M. Jones
2018-Oct-01 11:28 UTC
[Libguestfs] [PATCH API PROPOSAL 1/2] daemon: Allow OCaml code to raise Not_supported exception.
It would be possible to implement this as: raise (Unix_error (ENOTSUP, fn, arg)) except that Unix.ENOTSUP is not defined (OCaml 4.07). Instead of working around this let's have a special exception, and anyway this works like the C code. The only tricky part of this patch is that I wanted to use the ‘Daemon’ module name for daemonic things, such as this exception. So I had to rename the existing Daemon module (which contains initialization code) to ‘Daemon_init’. --- daemon/Makefile.am | 4 +++- daemon/daemon-c.c | 2 ++ daemon/daemon.ml | 21 ++++----------------- daemon/daemon.mli | 3 ++- daemon/daemon_init.ml | 38 ++++++++++++++++++++++++++++++++++++++ daemon/daemon_init.mli | 19 +++++++++++++++++++ 6 files changed, 68 insertions(+), 19 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 5d1c222db..d3adcb53e 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -270,6 +270,7 @@ SOURCES_MLI = \ chroot.mli \ daemon.mli \ daemon_config.mli \ + daemon_init.mli \ devsparts.mli \ file.mli \ filearch.mli \ @@ -301,6 +302,7 @@ SOURCES_MLI = \ SOURCES_ML = \ daemon_config.ml \ + daemon.ml \ utils.ml \ structs.ml \ optgroups.ml \ @@ -333,7 +335,7 @@ SOURCES_ML = \ inspect_fs.ml \ inspect.ml \ callbacks.ml \ - daemon.ml + daemon_init.ml BOBJECTS = $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx) diff --git a/daemon/daemon-c.c b/daemon/daemon-c.c index 533bf7ce7..b347ade81 100644 --- a/daemon/daemon-c.c +++ b/daemon/daemon-c.c @@ -59,6 +59,8 @@ guestfs_int_daemon_exn_to_reply_with_error (const char *func, value exn) String_val (Field (exn, 2)), String_val (Field (exn, 3))); } + else if (STREQ (exn_name, "Not_supported")) + reply_with_error_errno (ENOTSUP, "%s", String_val (Field (exn, 1))); else if (STREQ (exn_name, "Failure")) reply_with_error ("%s", String_val (Field (exn, 1))); else if (STREQ (exn_name, "Sys_error")) diff --git a/daemon/daemon.ml b/daemon/daemon.ml index f24d64a13..132bc6a21 100644 --- a/daemon/daemon.ml +++ b/daemon/daemon.ml @@ -18,21 +18,8 @@ open Printf -(* When guestfsd starts up, early on (after parsing the command line - * but not much else), it calls 'caml_startup' which runs all - * initialization code in the OCaml modules, including this one. - * - * Therefore this is where we can place OCaml initialization code - * for the daemon. - *) +exception Not_supported of string + let () - (* Connect the guestfsd [-v] (verbose) flag into 'verbose ()' - * used in OCaml code to print debugging messages. - *) - if Utils.get_verbose_flag () then ( - Std_utils.set_verbose (); - eprintf "OCaml daemon loaded\n%!" - ); - - (* Register the callbacks which are used to call OCaml code from C. *) - Callbacks.init_callbacks () + (* Register exceptions. *) + Callback.register_exception "Not_supported" (Not_supported "") diff --git a/daemon/daemon.mli b/daemon/daemon.mli index c893b4a1e..856364a78 100644 --- a/daemon/daemon.mli +++ b/daemon/daemon.mli @@ -16,4 +16,5 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. *) -(* Nothing is exported. *) +exception Not_supported of string +(** Like the [NOT_SUPPORTED] macro in [daemon.h]. *) diff --git a/daemon/daemon_init.ml b/daemon/daemon_init.ml new file mode 100644 index 000000000..f24d64a13 --- /dev/null +++ b/daemon/daemon_init.ml @@ -0,0 +1,38 @@ +(* guestfs-inspection + * Copyright (C) 2009-2018 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +open Printf + +(* When guestfsd starts up, early on (after parsing the command line + * but not much else), it calls 'caml_startup' which runs all + * initialization code in the OCaml modules, including this one. + * + * Therefore this is where we can place OCaml initialization code + * for the daemon. + *) +let () + (* Connect the guestfsd [-v] (verbose) flag into 'verbose ()' + * used in OCaml code to print debugging messages. + *) + if Utils.get_verbose_flag () then ( + Std_utils.set_verbose (); + eprintf "OCaml daemon loaded\n%!" + ); + + (* Register the callbacks which are used to call OCaml code from C. *) + Callbacks.init_callbacks () diff --git a/daemon/daemon_init.mli b/daemon/daemon_init.mli new file mode 100644 index 000000000..c893b4a1e --- /dev/null +++ b/daemon/daemon_init.mli @@ -0,0 +1,19 @@ +(* guestfsd + * Copyright (C) 2009-2018 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +(* Nothing is exported. *) -- 2.19.0.rc0
Richard W.M. Jones
2018-Oct-01 11:28 UTC
[Libguestfs] [PATCH API PROPOSAL 2/2] inspection: Add network interfaces to inspection data.
This adds network interfaces to inspection data. The data is returned as a flat list of structs containing a general overview of the interfaces, and a separate call returning a fairly arbitrary list of key/value parameters for each interface. The list of parameters is roughly modelled on the ifcfg-* files, although with some mapping of the data to make it easier to consume. Note this doesn't attempt a detailed model of all possible guest network configurations, which in particular cases could be extremely complex. This commits just throws ENOTSUP error for all guests. Following commits add this information for Red Hat-derived, Debian-derived and Windows guests. --- daemon/inspect.ml | 18 +++++ daemon/inspect_types.ml | 19 +++++- daemon/inspect_types.mli | 3 + generator/actions_inspection.ml | 86 ++++++++++++++++++++++++ generator/proc_nr.ml | 2 + generator/structs.ml | 10 +++ gobject/Makefile.inc | 2 + java/Makefile.inc | 1 + java/com/redhat/et/libguestfs/.gitignore | 1 + lib/MAX_PROC_NR | 2 +- 10 files changed, 142 insertions(+), 2 deletions(-) diff --git a/daemon/inspect.ml b/daemon/inspect.ml index ce62c17f2..0cb1aa5d8 100644 --- a/daemon/inspect.ml +++ b/daemon/inspect.ml @@ -376,6 +376,24 @@ and inspect_get_drive_mappings root let root = search_for_root root in root.inspection_data.drive_mappings +and inspect_get_interfaces root + let root = search_for_root root in + match root.inspection_data.interfaces with + | None -> + raise (Daemon.Not_supported "guest type not supported") + | Some interfaces -> List.map fst interfaces + +and inspect_get_interface_parameters root i + let root = search_for_root root in + match root.inspection_data.interfaces with + | None -> + raise (Daemon.Not_supported "guest type not supported") + | Some interfaces -> + let interfaces = List.map snd interfaces in + try List.nth interfaces i + with Failure _ | Invalid_argument _ -> + failwith "index out of range" + and search_for_root root let fses = !Inspect_types.inspect_fses in if fses = [] then diff --git a/daemon/inspect_types.ml b/daemon/inspect_types.ml index 70b34c51c..37592275b 100644 --- a/daemon/inspect_types.ml +++ b/daemon/inspect_types.ml @@ -54,6 +54,7 @@ and inspection_data = { mutable windows_system_hive : string option; mutable windows_current_control_set : string option; mutable drive_mappings : drive_mapping list; + mutable interfaces : interfaces option; } and os_type | OS_TYPE_DOS @@ -125,6 +126,8 @@ and package_management and version = int * int and fstab_entry = Mountable.t * string (* mountable, mountpoint *) and drive_mapping = string * string (* drive name, device *) +and interfaces = interface list +and interface = Structs.interface * (string * string) list let rec string_of_fs { fs_location = location; role } sprintf "fs: %s role: %s\n" @@ -183,6 +186,17 @@ and string_of_inspection_data data List.map (fun (a, b) -> sprintf "(%s, %s)" a b) data.drive_mappings in bpf " drive_mappings: [%s]\n" (String.concat ", " v) ); + Option.may ( + fun interfaces -> + bpf " interfaces:\n"; + List.iteri ( + fun i ({ Structs.if_type; if_hwaddr; if_name }, params) -> + bpf " [%d]: type: %s mac: %s name: %s" + i if_type if_hwaddr if_name; + List.iter (fun (k, v) -> bpf " %s: %s" k v) params; + bpf "\n" + ) interfaces + ) data.interfaces; Buffer.contents b and string_of_os_type = function @@ -272,6 +286,7 @@ let null_inspection_data = { windows_system_hive = None; windows_current_control_set = None; drive_mappings = []; + interfaces = None; } let null_inspection_data () = { null_inspection_data with os_type = None } @@ -299,7 +314,9 @@ let merge_inspection_data child parent merge child.windows_current_control_set parent.windows_current_control_set; (* This is what the old C code did, but I doubt that it's correct. *) - parent.drive_mappings <- child.drive_mappings @ parent.drive_mappings + parent.drive_mappings <- child.drive_mappings @ parent.drive_mappings; + + parent.interfaces <- merge child.interfaces parent.interfaces let merge child_fs parent_fs let inspection_data_of_fs = function diff --git a/daemon/inspect_types.mli b/daemon/inspect_types.mli index 7493aa3a6..e82b2a554 100644 --- a/daemon/inspect_types.mli +++ b/daemon/inspect_types.mli @@ -57,6 +57,7 @@ and inspection_data = { mutable windows_system_hive : string option; mutable windows_current_control_set : string option; mutable drive_mappings : drive_mapping list; + mutable interfaces : interfaces option; } (** During inspection, this data is collected incrementally for each filesystem. At the end of inspection, inspection data is merged @@ -132,6 +133,8 @@ and package_management and version = int * int and fstab_entry = Mountable.t * string (* mountable, mountpoint *) and drive_mapping = string * string (* drive name, device *) +and interfaces = interface list +and interface = Structs.interface * (string * string) list val merge_inspection_data : inspection_data -> inspection_data -> unit (** [merge_inspection_data child parent] merges two sets of inspection diff --git a/generator/actions_inspection.ml b/generator/actions_inspection.ml index d913f53e3..a4ca437ff 100644 --- a/generator/actions_inspection.ml +++ b/generator/actions_inspection.ml @@ -603,6 +603,92 @@ Please read L<guestfs(3)/INSPECTION> for more details. See also C<guestfs_inspect_get_mountpoints>, C<guestfs_inspect_get_filesystems>." }; + { defaults with + name = "inspect_get_interfaces"; added = (1, 39, 11); + style = RStructList ("interfaces", "interface"), [String (Mountable, "root")], []; + impl = OCaml "Inspect.inspect_get_interfaces"; + shortdesc = "get the list of network interfaces"; + longdesc = "\ +Return the list of network interfaces seen by this guest. This returns +a list of structs, one struct per network interface. + +The returned C<interface> struct contains only a few fields common +to all interfaces. For detailed parameters for each interface +you also need to call C<guestfs_inspect_get_interface_parameters> +with the index of the interface you are interested in. + +The returned C<interface> struct contains these fields: + +=over 4 + +=item C<if_type> + +Interface type, including C<ethernet> (wired ethernet), C<wireless> +(WiFi). Other interface types are possible. + +=item C<if_hwaddr> + +The hardware address (MAC address) of the interface if one is known, +otherwise the empty string. + +=item C<if_name> + +The name of the interface as it is known to the guest (eg. C<eth0>). +Note that network interface names might not be unique. + +=back + +Depending on the guest type this call may return more network interfaces +than the guest has physical devices. In particular it may return all +network interfaces ever seen by the guest across multiple boots. To +work out which interfaces correspond to currently available devices +you will have to use MAC address information derived from another +source (eg. hypervisor metadata) and match that to C<if_hwaddr> from +the struct. + +If inspection of network interfaces is not implemented for this +particular guest type then this will fail with an error and set +C<errno> to C<ENOTSUP>, so callers should be careful to check for that +case." }; + + { defaults with + name = "inspect_get_interface_parameters"; added = (1, 39, 11); + style = RHashtable (RPlainString, RPlainString, "parameters"), [String (Mountable, "root"); Int "index"], []; + impl = OCaml "Inspect.inspect_get_interface_parameters"; + shortdesc = "get detailed configuration parameters for a network interface"; + longdesc = "\ +Return the detailed configuration parameters of the C<index> network +interface. Indexes refer to the list returned by +C<guestfs_inspect_get_interfaces> and count from zero. + +The returned parameters can contain a variety of keys. Some notable +ones are described below: + +=over 4 + +=item C<bootproto> + +C<dhcp> if the interface was last configured to find an IP address +through DHCP, else C<none> if it was configured to use a static +IP address. Other values may be possible. + +=item C<essid> + +For wireless interfaces, the ESSID. + +=item C<ipaddr>, C<network>, C<netmask>, C<gateway>, C<broadcast> + +IPv4 address if configured statically. For some (but not all) +guest types if the interface uses DHCP then this may record the +previous IPv4 address which was acquired through DHCP. + +=item C<onboot> + +Key is present and value is C<1> if the network interface is configured +to come up at boot. + +=back" }; + ] let non_daemon_functions = [ diff --git a/generator/proc_nr.ml b/generator/proc_nr.ml index 2adf8a32f..4781f0a24 100644 --- a/generator/proc_nr.ml +++ b/generator/proc_nr.ml @@ -514,6 +514,8 @@ let proc_nr = [ 504, "part_get_gpt_attributes"; 505, "f2fs_expand"; 506, "lvm_scan"; +507, "inspect_get_interfaces"; +508, "inspect_get_interface_parameters"; ] (* End of list. If adding a new entry, add it at the end of the list diff --git a/generator/structs.ml b/generator/structs.ml index 14ee2813f..09e90bd0b 100644 --- a/generator/structs.ml +++ b/generator/structs.ml @@ -478,6 +478,16 @@ let structs = [ ]; s_camel_name = "YaraDetection" }; + (* Network interfaces. *) + { defaults with + s_name = "interface"; + s_cols = [ + "if_type", FString; + "if_hwaddr", FString; + "if_name", FString; + ]; + s_camel_name = "Interface" }; + ] (* end of structs *) let lookup_struct name diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc index 5aa2dcafe..42839a85f 100644 --- a/gobject/Makefile.inc +++ b/gobject/Makefile.inc @@ -35,6 +35,7 @@ guestfs_gobject_headers= \ include/guestfs-gobject/struct-hivex_value.h \ include/guestfs-gobject/struct-inotify_event.h \ include/guestfs-gobject/struct-int_bool.h \ + include/guestfs-gobject/struct-interface.h \ include/guestfs-gobject/struct-isoinfo.h \ include/guestfs-gobject/struct-lvm_lv.h \ include/guestfs-gobject/struct-lvm_pv.h \ @@ -128,6 +129,7 @@ guestfs_gobject_sources= \ src/struct-hivex_value.c \ src/struct-inotify_event.c \ src/struct-int_bool.c \ + src/struct-interface.c \ src/struct-isoinfo.c \ src/struct-lvm_lv.c \ src/struct-lvm_pv.c \ diff --git a/java/Makefile.inc b/java/Makefile.inc index 93ec5e129..54b9da971 100644 --- a/java/Makefile.inc +++ b/java/Makefile.inc @@ -33,6 +33,7 @@ java_built_sources = \ com/redhat/et/libguestfs/INotifyEvent.java \ com/redhat/et/libguestfs/ISOInfo.java \ com/redhat/et/libguestfs/IntBool.java \ + com/redhat/et/libguestfs/Interface.java \ com/redhat/et/libguestfs/LV.java \ com/redhat/et/libguestfs/MDStat.java \ com/redhat/et/libguestfs/PV.java \ diff --git a/java/com/redhat/et/libguestfs/.gitignore b/java/com/redhat/et/libguestfs/.gitignore index bc03cb965..b5843c8f1 100644 --- a/java/com/redhat/et/libguestfs/.gitignore +++ b/java/com/redhat/et/libguestfs/.gitignore @@ -10,6 +10,7 @@ HivexValue.java INotifyEvent.java ISOInfo.java IntBool.java +Interface.java LV.java MDStat.java PV.java diff --git a/lib/MAX_PROC_NR b/lib/MAX_PROC_NR index 80e3e6eab..1eccde110 100644 --- a/lib/MAX_PROC_NR +++ b/lib/MAX_PROC_NR @@ -1 +1 @@ -506 +508 -- 2.19.0.rc0
Pino Toscano
2018-Oct-01 17:00 UTC
Re: [Libguestfs] [PATCH API PROPOSAL 0/2] inspection: Add network interfaces to inspection data.
On Monday, 1 October 2018 13:28:26 CEST Richard W.M. Jones wrote:> As part of the fix for: > > https://bugzilla.redhat.com/show_bug.cgi?id=1626503 > > I'm proposing to add two new APIs to fetch information about the list > of network interfaces of an existing guest. These two patches outline > the proposed API but with no implementation or tests. However they > can be applied and compiled. > > Please see the second patch for the proposed API. > > I have a mostly working implementation for Red Hat-style ifcfg-* > files, and both Debian /etc/network/interfaces and Windows registry > look do-able using the same architecture.It is hard to see whether this API is enough to cover the use case, without an actual implementation. I remember RFCs for virt-builder/virt-customize to change the network configuration of a guest, and even just determining the actual configuration that a guest will have is very hard: you can have multiple network-related daemon installed in the guest, different available configurations (even stale ones, if they were not cleaned when their system was removed), and so on. Hence, I am notsure whether trying to get any data is better than just not provide any at all... -- Pino Toscano
Richard W.M. Jones
2018-Oct-02 09:10 UTC
Re: [Libguestfs] [PATCH API PROPOSAL 0/2] inspection: Add network interfaces to inspection data.
On Mon, Oct 01, 2018 at 07:00:17PM +0200, Pino Toscano wrote:> On Monday, 1 October 2018 13:28:26 CEST Richard W.M. Jones wrote: > > As part of the fix for: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1626503 > > > > I'm proposing to add two new APIs to fetch information about the list > > of network interfaces of an existing guest. These two patches outline > > the proposed API but with no implementation or tests. However they > > can be applied and compiled. > > > > Please see the second patch for the proposed API. > > > > I have a mostly working implementation for Red Hat-style ifcfg-* > > files, and both Debian /etc/network/interfaces and Windows registry > > look do-able using the same architecture. > > It is hard to see whether this API is enough to cover the use case, > without an actual implementation. > > I remember RFCs for virt-builder/virt-customize to change the network > configuration of a guest, and even just determining the actual > configuration that a guest will have is very hard: you can have > multiple network-related daemon installed in the guest, different > available configurations (even stale ones, if they were not cleaned > when their system was removed), and so on. > Hence, I am notsure whether trying to get any data is better than just > not provide any at all...I posted v2 of this patch last night which includes a single working implementation, but I also meant to reply to this message with some more detail. I'm not expecting we'll be able to model every detail of guest network configuration. However a simple list of interfaces is useful for several purposes: * The virt-v2v bug (see above). * Answering the "what IP does the guest have" question which comes up again and again. * A way to tie external devices presented by the hypervisor to internal device drivers provided by the guest. For Linux it's true that multiple network services can be installed, although only one is likely to be used and it's not too much work to find out which one is used (although the current patch set does punt on this question and makes some assumptions). For Windows the answer is much clearer and simpler: there's just a list of network devices in the registry and that's what we present through the inspection data. Another thing which may not be clear from the patch posted: The list of parameters should be canonicalized, so that for example the Windows registry key "IPAddress" is translated to "ipaddr". In all cases I'll try to use the name of the thing used by the ifcfg-* files. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Reasonably Related Threads
- [PATCH v2 API PROPOSAL 0/5] inspection Add network interfaces to inspection data.
- [PATCH] UNFINISHED daemon: Reimplement most inspection APIs in the daemon.
- [PATCH v10 00/10] Reimplement inspection in the daemon.
- [PATCH v9 00/11] Reimplement inspection in the daemon.
- [PATCH v11 00/10] Reimplement inspection in the daemon.