Re: [Libguestfs] [PATCH v2v 3/5] -o libvirt: Add to libvirt XML
On Mon, Sep 25, 2023 at 05:47:26PM +0200, Laszlo Ersek wrote: > On 9/25/23 16:04, Richard W.M. Jones wrote: > > --- > > output/create_libvirt_xml.ml | 10 -- > > tests/test-v2v-i-ova.xml | 1 + > > 2 files changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml > > index 964acd25fd..f97272ca31 100644 > > --- a/output/create_libvirt_xml.ml > > +++ b/output/create_libvirt_xml.ml > > @@ -292,10 +292,16 @@ let create_libvirt_xml ?pool source inspect > > e "nvram" ["template", vars_template] [] ] in > > > > List.push_back_list os loader; > > -!os in > > +e "os" [] !os in > > + > > + (* The section. *) > > + let clock_section = > > +let offset = if guestcaps.gcaps_bios_utc then "utc" else "localtime" in > > +e "clock" [ "offset", offset ] [] in > > > >List.push_back_list body [ > > -e "os" [] os_section; > > +os_section; > > +clock_section; > > > > e "on_poweroff" [] [PCData "destroy"]; > > e "on_reboot" [] [PCData "restart"]; > > I don't think we need to change what "os_section" is bound to, here; I > understand the idea is to increase consistency, but for me it makes the > patch harder to read -- ultimately it is a refactoring, and then adding > in clock_section is the new thing. > > If you can split these apart, that's optimal; if not, this is viable too > IMO. Yup, that is a separate clean-up, will split. Rich. > Thanks > Laszlo > > > diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml > > index e5907ea1cc..a41827bfd5 100644 > > --- a/tests/test-v2v-i-ova.xml > > +++ b/tests/test-v2v-i-ova.xml > > @@ -18,6 +18,7 @@ > > > > hvm > > > > + > >destroy > >restart > >restart -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v2v 3/5] -o libvirt: Add to libvirt XML
On 9/25/23 16:04, Richard W.M. Jones wrote: > --- > output/create_libvirt_xml.ml | 10 -- > tests/test-v2v-i-ova.xml | 1 + > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml > index 964acd25fd..f97272ca31 100644 > --- a/output/create_libvirt_xml.ml > +++ b/output/create_libvirt_xml.ml > @@ -292,10 +292,16 @@ let create_libvirt_xml ?pool source inspect > e "nvram" ["template", vars_template] [] ] in > > List.push_back_list os loader; > -!os in > +e "os" [] !os in > + > + (* The section. *) > + let clock_section = > +let offset = if guestcaps.gcaps_bios_utc then "utc" else "localtime" in > +e "clock" [ "offset", offset ] [] in > >List.push_back_list body [ > -e "os" [] os_section; > +os_section; > +clock_section; > > e "on_poweroff" [] [PCData "destroy"]; > e "on_reboot" [] [PCData "restart"]; I don't think we need to change what "os_section" is bound to, here; I understand the idea is to increase consistency, but for me it makes the patch harder to read -- ultimately it is a refactoring, and then adding in clock_section is the new thing. If you can split these apart, that's optimal; if not, this is viable too IMO. Thanks Laszlo > diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml > index e5907ea1cc..a41827bfd5 100644 > --- a/tests/test-v2v-i-ova.xml > +++ b/tests/test-v2v-i-ova.xml > @@ -18,6 +18,7 @@ > > hvm > > + >destroy >restart >restart ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v2v 3/5] -o libvirt: Add to libvirt XML
--- output/create_libvirt_xml.ml | 10 -- tests/test-v2v-i-ova.xml | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 964acd25fd..f97272ca31 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -292,10 +292,16 @@ let create_libvirt_xml ?pool source inspect e "nvram" ["template", vars_template] [] ] in List.push_back_list os loader; -!os in +e "os" [] !os in + + (* The section. *) + let clock_section = +let offset = if guestcaps.gcaps_bios_utc then "utc" else "localtime" in +e "clock" [ "offset", offset ] [] in List.push_back_list body [ -e "os" [] os_section; +os_section; +clock_section; e "on_poweroff" [] [PCData "destroy"]; e "on_reboot" [] [PCData "restart"]; diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml index e5907ea1cc..a41827bfd5 100644 --- a/tests/test-v2v-i-ova.xml +++ b/tests/test-v2v-i-ova.xml @@ -18,6 +18,7 @@ hvm + destroy restart restart -- 2.41.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
