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

[Richard W.M. Jones]

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 at redhat.com>
> >>---
> >> 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