Re: [Libguestfs] [PATCH] v2v: Add support for libosinfo metadata

On Fri, Nov 23, 2018 at 02:10:01PM +0100, Martin Kletzander wrote: > On Fri, Nov 23, 2018 at 11:53:05AM +0000, Richard W.M. Jones wrote: > >On Fri, Nov 23, 2018 at 12:39:44PM +0100, Martin Kletzander wrote: > >>There's a standardized libosinfo namespace for libvirt domain metadata. For now > >>it supports the id of the OS only. However that is still a very helpful feature > >>that is already supported in gnome-boxes and virt-manager (at least). > >> > >>The discussion happened here: > >> > >> https://www.redhat.com/archives/libosinfo/2018-September/msg00003.html > > > >It's a shame I missed this thread because I'd probably have asked why > >we're inventing yet another scheme for naming guests. Particularly in > >the libosinfo project which has already got the canonical naming > >scheme! > > > >Anyway that's water under the bridge now so ... > > > > But it is using the osinfo naming scheme. I mean the ID. Not sure I understand. osinfo currently has osinfo IDs like say "rhel7.6", but now we've invented a new scheme "http://redhat.com/rhel/7.6".

In any case this isn't the place to argue about the merits of the new scheme, since it seems to have been settled in libosinfo and libvirt. > The matching code could be made shorter, but since this is my > "tryout patch" for libguestfs, I'm just glad it works. I'd > appreciate literally *any* comment to any part of this. Apart from the duplicate case (which is in fact not duplicated), I didn't have any other problem. However ... > I haven't find any unit tests for these kind of functions, so no > tests are added. If there is a place where tests would fit > nicely, feel free to let me know. ... I think the tests might actually be broken by this patch. Did you try: ‘make -C test-data check && make -C v2v check’?

> >>So let's add the support to local and libvirt outputs. > >> > >>Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > >>--- > >> v2v/create_libvirt_xml.ml | 109 ++++++++++++++++++++++++++++++++++++- > >> v2v/create_libvirt_xml.mli | 1 + > >> v2v/output_libvirt.ml | 4 +- > >> v2v/output_local.ml | 4 +- > >> 4 files changed, 113 insertions(+), 5 deletions(-) > >> > >>diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml > >>index 55e83e8bc1b9..180f3768792b 100644 > >>--- a/v2v/create_libvirt_xml.ml > >>+++ b/v2v/create_libvirt_xml.ml > >>@@ -34,8 +34,102 @@ let find_target_disk targets { s_disk_id = id } = > >> try List.find (fun t -> t.target_overlay.ov_source.s_disk_id = id) targets > >> with Not_found -> assert false > >> > >>+let get_osinfo_id = function > >>+ | { i_type = "linux"; i_distro = "rhel"; > >>+ i_major_version = major; i_minor_version = minor } -> > >>+ Some (sprintf "http://redhat.com/rhel/%d.%d" major minor) > >>+ > >>+ | { i_type = "linux"; i_distro = "centos"; > >>+ i_major_version = major; i_minor_version = minor } when major < 7 -> > >>+ Some (sprintf "http://centos.org/centos/%d.%d" major minor) > >>+ > >>+ | { i_type = "linux"; i_distro = "centos"; i_major_version = major } -> > >>+ Some (sprintf "http://centos.org/centos/%d.0" major) > >>+ > >>+ | { i_type = "linux"; i_distro = "sles"; > >>+ i_major_version = major; i_minor_version = 0 } -> > >>+ Some (sprintf "http://suse.com/sles/%d" major) > >>+ > >>+ | { i_type = "linux"; i_distro = "sles"; > >>+ i_major_version = major; i_minor_version = minor } -> > >>+ Some (sprintf "http://suse.com/sles/%d.%d" major minor) > > > >SLES is duplicated? > > > > Oh, I removed the comment i had there. osinfo does not use 0 in the version, > it's just SLES 10, SLES 10.1 (for SP1), 10.2 (for SP2) and so on. Oh I see now, it's not duplicated because of the check for i_minor_version = 0. > But I just noticed I forgot to differentiate SLED from SLES. It > should be possible based on i_product_name. I think it would make > sense for libosinfo and it doesn't look like they are separate in > libguestfs (on purpose). Would it also make sense to separate them > here? Is it even possible? I'm not sure what the compatibility > promises are. No idea. We just try to provide libvirt XML which is as useful as possible. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top