Description of problem: I have a working v2v appliance. I am doing conversion with output to openstack as cinder volume. ie, -o openstack -oo server-id="id-of-v2v-appliance" so all the converted data will be created as a cinder volume attached to v2v appliance. I am trying to do p2v from a windows physical machine using the same v2v appliance and p2v bootable media created from fedora. But, there is no feature in p2v program to give options -oo server-id. I tried to give it as kernel argument, p2v.oo='serverid=sadaasd' But, no luck. The v2v appliance is throwing error: virt-v2v: error: openstack: -oo server-id=<NAME|UUID> not present The v2v supports the featuire but p2v doesnt. Looks like lack of feature in p2v program Version-Release number of selected component (if applicable): How reproducible: Build a p2v appliance from fedora, boot it on any physical machine. Try to give output as openstack (p2v.o=openstack) Steps to Reproduce: 1. Build p2v bootable media from fedora 2. boot p2v and give output as 'openstack' 3. Actual results: virt-v2v: error: openstack: -oo server-id=<NAME|UUID> not present Expected results: It should have feature to allow -oo server-id='v2v-appliance-id' parameter. Additional info: The v2v appliance is on Fedora release 30
Sorry for the late answer. The biggest issue is that the openstack output mode requires some output options (-oo), and currently the virt-p2v interface does not provide a way to specify them, as you also noticed. The rhv-upload output mode has the same issue, and because of that it is not available in the virt-p2v interface. Hence, I submitted a patch to not show openstack as available output mode for now: https://www.redhat.com/archives/libguestfs/2020-March/msg00127.html A better fix for this will be to allow output options to be specified by the user; most probably it is about adding few UI knobs. The second issue you notice is that there is no way to specify output options via kernel command line arguments. This is slightly more complicated though.
Hello I'm using --cmdline to push parameter and OpenStack is not ignored in this way. I got also the same error "virt-v2v: error: openstack: -oo server-id=<NAME|UUID> not present" Is it possible to add p2v.oo parameter in virt-p2v as you mentioned with a list of parameters defined by the user ? Even if it's only compatible with cmdline and kernel arguments ? The responsibility to define valid parameters in p2v.oo section will be on the user side. For cmdline, I guess we need to modify "generate-p2v-config.pl" file to add : ["p2v.output.openstack", "p2v.oo"] in my @cmdline_aliases and add "p2v.output.openstack" => manual_entry->new( with the list -oo parameters like "server-id=xxx verify-server-certificate=false os-username=admin" I'm using virt-p2v 1.42.2fedora release=1.1.el9 and virt-v2v 2.0.7rhel=9,release=7.el9 Thanks
Created attachment 1937412 [details] Proposed Change to add -oo in cli and kernelcmd Tested in cmdline and it works
(In reply to alecorps from comment #3) > Created attachment 1937412 [details] > Proposed Change to add -oo in cli and kernelcmd > > Tested in cmdline and it works Example of cmdline for the test: /usr/bin/virt-p2v --cmdline='p2v.server=xxx p2v.username=root p2v.os=ssd p2v.identity=file:///var/tmp/id_rsa p2v.sudo=true p2v.guestname=xxx p2v.o=openstack p2v.oo="server-id=xxxx,os-auth-url=https://xxx.com:5000/v3,os-password=xxx,os-username=alecorps,os-project-name=xxx,os-region-name=xxx,os-project-domain-id=default,os-user-domain-name=Default"'
I'll investigate (I'm taking the BZ so I don't forget about it).
Created attachment 1937790 [details] generate-p2v-config.pl
Created attachment 1937791 [details] modification of conversion.c to add -oo
Please post patches rather than whole files, and also you can send them (without subscribing) to libguestfs. Ideally, use git-email. A couple of thoughts on this bug in general: virt-p2v tries to model virt-v2v output modes through the GUI and configuration. This means that new output modes are difficult to add, especially if they involve lots of custom configuration options, and -oo openstack definitely falls into this category. There's some case for allowing a free-form "extra virt-v2v parameters" field. This would let people use virt-p2v for a particular output mode even before we have fully modelled the extra options in the GUI. For some unusual options we might even leave it at that.
(1) The -oo options are output-specific. Therefore, mapping them to the GUI is a problem. The GUI currently side-steps this by adding helpful tooltips (with gtk_widget_set_tooltip_markup()) to the -o, -oc, -os, -of entries. In other words, the options we currently expose are already output-specific, we just tell the user what option to ignore for what output module. This (= tooltips) can work the same with -oo, so consistency would not be improved or deteriorated. (2) The -oo option is extra messy. That's because multiple -oo options can be set, and especially in case of the openstack output, even virt-v2v uses a *passthrough* scheme, for handling -oo os-*=*. See virt-v2v commits 54965103b2c7 ("v2v: Add -o openstack target, writes to OpenStack & Cinder using APIs.", 2018-09-03) and 7e81c67eff03 ("v2v: -o openstack: Restrict passthrough auth params to ‘os-*’.", 2018-11-20). Now, from comment 6 and comment 7, I kind of "reconstructed" a unified diff on top of virt-p2v commit f4c7ae6ba98f ("make-disk: inject EPEL9 repo in RHEL9 disk image", 2023-01-20). See it later, below. It seems reasonable that the patch attempts to reuse the ConfigStringList facility, for (effectively) turning a single text entry into a comma-separated list of options. The problem (or, well, inconsistency) with that is however that all the ConfigStringList knobs are currently represented with actual list widgets on the GUI. Exposing a list (of unspecified length!) of text entries would be a major complication for the GUI, whereas exposing a single new text entry that would later be split at commas would be inconsistent with the rest of the GUI. (3) There's some historical inconsistency around output drivers. Commit 7a61a30f142c ("p2v: Add GUI controls for -o, -oc etc options on virt-v2v command line.", 2014-09-02) introduced the text entries originally; it open-coded a number of output modes, including open-coding "OpenStack Glance" in the "-o" drop down list tooltip. Later, commit cff9e846693c ("p2v/v2v: List output drivers in `virt-v2v --machine-readable', and use that in virt-p2v.", 2014-09-02) replaced the open-coded output module list with dynamic fetching from virt-v2v, *but* the tooltip remained statick. Yet later, commit b74c126629e3 ("Ignore 'openstack' driver", 2020-03-16) would filter "openstack" out of said dynamic fetching, but we still have the "glance" reference in the tooltip. (I realize that virt-v2v's "-o glance" and "-o openstack" differ; still, it remains a confusing reference in virt-p2v.) (4) The patch then, reconstructed: > diff --git a/generate-p2v-config.pl b/generate-p2v-config.pl > index 47487f7b0f35..0dc0f50491be 100755 > --- a/generate-p2v-config.pl > +++ b/generate-p2v-config.pl > @@ -147,6 +147,7 @@ my @fields = [ > ConfigString->new(name => 'connection'), > ConfigString->new(name => 'format'), > ConfigString->new(name => 'storage'), > + ConfigStringList->new(name => 'openstack'), > ], > ), > ]; The naming here ("openstack") is too specific. According to <https://libguestfs.org/virt-v2v.1.html#options> as well, "-oo" generally exists, only its interpretation is up to each output mode ("Set output option(s) related to the current output mode"). > @@ -168,6 +169,7 @@ my @cmdline_aliases = ( > ["p2v.output.connection", "p2v.oc"], > ["p2v.output.format", "p2v.of"], > ["p2v.output.storage", "p2v.os"], > + ["p2v.output.openstack", "p2v.oo"], > ); > > # Some config entries are not exposed on the kernel command line. This hunk should not be necessary; as the leading comment in the (invisible) context says, "cmdline_aliases" is a compat layer. It was introduced so that old (public) knobs would continue working after the knobs were renamed. For newly introduced options, compatibility is not required (there is no "old" public name to recognize). > @@ -357,6 +359,12 @@ written to F</var/tmp>.", > description => " > Set the output allocation mode. This is the same as the virt-v2v > I<-oa> option. See L<virt-v2v(1)/OPTIONS>.", > + ), > + "p2v.output.openstack" => manual_entry->new( > + shortopt => "", # ignored for enums > + description => " > +Set the output openstack. This is the same as the virt-v2v > +I<-oo> option. See L<virt-v2v(1)/OPTIONS>.", > ), > "p2v.output.connection" => manual_entry->new( > shortopt => "URI", Again, this language should be more generic. A hint at openstack could be useful (which is an output module that heavily relies on -oo). > diff --git a/conversion.c b/conversion.c > index b9af47deda74..286a4fe84c80 100644 > --- a/conversion.c > +++ b/conversion.c > @@ -502,6 +502,14 @@ generate_wrapper_script (struct config *config, const char *remote_dir, > print_quoted (fp, config->output.type); > } > > + if (config->output.openstack) { /* -oo */ > + int i; > + for (i = 0; config->output.openstack[i] != NULL; ++i) { > + fprintf (fp, " -oo "); > + print_quoted (fp, config->output.openstack[i]); > + } > + } > + > switch (config->output.allocation) { /* -oa */ > case OUTPUT_ALLOCATION_NONE: > /* nothing */ I think this hunk is nice, (again) relying on ConfigStringList. IMO the main problem is how we represent such a list on the GUI whose length is to be controlled by the user. That's a first -- it's inherently incompatible with our current approach, namely that of highlighting the "main" -o[...] options individually. With "-oo", the utility of individually enumerating output options on the GUI mostly vanishes. And, in any case, the documentation (with the screenshots / diagrams) would have to be updated. (I've mostly only elaborated on what Rich already said in comment#8; in a way I needed to explain it to myself.) I can look into the "split at commas" idea later. It should probably be done in start_conversion_clicked(), where we already "Unpack dialog fields and check them"; the important thing is to split with the same logic as the kernel cmdline parser does.
I believe I was wrong about the Glance comment being outdated, from commit 7a61a30f142c. Upstream virt-v2v supports glance [*], and -o glance doesn't seem to require (or even to handle) -oo. Therefore, the comment remains relevant. [*] downstream doesn't, per bug 1977539; it's patched out of virt-v2v, and so virt-p2v doesn't have to filter out "glance" -- it's simply not there in the "--machine-readable" list of "output:..." lines from virt-v2v.
-o glance is quite a simple output mode. I believe it requires no(!) additional options, because it just runs the "glance" program and all configuration is done using environment variables that glance understands (or probably some config file). It is indeed both discouraged upstream and removed in RHEL because glance (templates / cattle) doesn't fit with the virt-v2v model (pets).
(In reply to Laszlo Ersek from comment #9) > > @@ -168,6 +169,7 @@ my @cmdline_aliases = ( > > ["p2v.output.connection", "p2v.oc"], > > ["p2v.output.format", "p2v.of"], > > ["p2v.output.storage", "p2v.os"], > > + ["p2v.output.openstack", "p2v.oo"], > > ); > > > > # Some config entries are not exposed on the kernel command line. > > This hunk should not be necessary; as the leading comment in the > (invisible) context says, "cmdline_aliases" is a compat layer. It was > introduced so that old (public) knobs would continue working after the > knobs were renamed. For newly introduced options, compatibility is not > required (there is no "old" public name to recognize). I'm revising this too -- while the compat knob is not required, the "p2v.oo" shorthand is simply more comfortable to write.
The splitting of ConfigStringList, from the kernel cmdline, is performed with guestfs_int_split_string() -- copied from libguestfs, into "libguestfs/guestfs-utils.c" --, in the generated file "kernel-config.c". This means two things: - guestfs_int_split_string() should be usable when the knob is updated from the GUI (simple text entry), - when generate_wrapper_script() is reached on the "headless" path, the resultant string list can indeed be NULL. guestfs_int_split_string() always creates at least one element in the output array (= the terminating NULL element), but in case the new param is not passed on the kernel cmdline, guestfs_int_split_string() is not called by the generated code at all, in my understanding, so the array itself can be NULL.
[p2v PATCH 00/11] Expose virt-v2v's "-oo"; re-enable openstack Message-Id: <20230130142228.108135-1-lersek> https://listman.redhat.com/archives/libguestfs/2023-January/030539.html
[p2v PATCH v2 00/11] Expose virt-v2v's "-oo"; re-enable openstack Message-Id: <20230202102928.21597-1-lersek> https://listman.redhat.com/archives/libguestfs/2023-February/030588.html
Upstream commit range f4c7ae6ba98f..62e92514e11f. Because this BZ was filed for the upstream p2v project, I'm closing it as resolved now.