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

[Martin Kletzander]

On Fri, Nov 23, 2018 at 01:16:55PM +0000, Richard W.M. Jones wrote:
>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".
>

It's not new.  Libosinfo has support for both.  The first one, "rhel7.6" in this
case, is a "short id", where the second one is actually called an "ID".

Example output of `osinfo-query -f short-id,id,name os short-id=rhel7.4`
(truncated) should explain:

  rhel7.4 | Red Hat Enterprise Linux 7.4 | http://redhat.com/rhel/7.4

>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’?
>

good point, probably not after the patch. will do.

>> >>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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20181123/1a0c01ad/attachment.sig>