On Mon, Mar 15, 2021 at 05:49:08PM +0200, Sam Eiderman wrote: > On Mon, Mar 15, 2021 at 12:38 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > > > On Sun, Mar 14, 2021 at 05:33:42PM +0200, Sam Eiderman wrote: > > > By using: > > > > > > export LIBGUESTFS_BACKEND_SETTINGS=force_kvm > > > > > > you can force the backend to use KVM and never fall back to > > > TCG (software emulation). > > > --- > > > lib/launch-direct.c | 22 +++++++++++++++++++--- > > > lib/launch-libvirt.c | 12 +++++++++++- > > > 2 files changed, 30 insertions(+), 4 deletions(-) > > > > > > diff --git a/lib/launch-direct.c b/lib/launch-direct.c > > > index b6ed9766f..79e6ef2fd 100644 > > > --- a/lib/launch-direct.c > > > +++ b/lib/launch-direct.c > > > @@ -385,6 +385,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) > > > struct hv_param *hp; > > > bool has_kvm; > > > int force_tcg; > > > + int force_kvm; > > > > Wouldn't this be better as a tri-state setting? eg: > > > > enum { BEST_ACCEL, FORCE_TCG, FORCE_KVM } accel; > > > > It seems it would make the checks below simpler. > > > > Indeed this is the mindset, but not sure it will change much in code. > Parsing will have to populate that enum anyway so we will have to > check that both values do not collide. I don't necessarily have strong opinions one way or the other, but I guess one variable is better than two? Rich. > > > > const char *cpu_model; > > > CLEANUP_FREE char *append = NULL; > > > CLEANUP_FREE_STRING_LIST char **argv = NULL; > > > @@ -434,8 +435,22 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) > > > if (force_tcg == -1) > > > return -1; > > > > > > - if (!has_kvm && !force_tcg) > > > - debian_kvm_warning (g); > > > + force_kvm = guestfs_int_get_backend_setting_bool (g, "force_kvm"); > > > + if (force_kvm == -1) > > > + return -1; > > > + > > > + if (force_kvm && force_tcg) { > > > + error (g, "Both force_kvm and force_tcg backend settings supplied."); > > > + return -1; > > > + } > > The following if statement will stay the same probably (just use the > enum values) - notice that we fallthrough in printing the warning. > > > > + if (!has_kvm) { > > > + if (!force_tcg) > > > + debian_kvm_warning (g); > > > + if (force_kvm) { > > > + error (g, "force_kvm supplied but kvm not available."); > > > + return -1; > > > + } > > > + } > > > > > > /* Using virtio-serial, we need to create a local Unix domain socket > > > * for qemu to connect to. > > > @@ -530,7 +545,8 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) > > > if (has_kvm && !force_tcg) > > > append_list ("gic-version=host"); > > > #endif > > This might be slightly simplified - but not sure I would add an enum > for that, no strong opinions though. > > > > - append_list_format ("accel=%s", !force_tcg ? "kvm:tcg" : "tcg"); > > > + append_list_format ("accel=%s", force_kvm ? "kvm" : > > > + (!force_tcg ? "kvm:tcg" : "tcg")); > > > } end_list (); > > > > > > cpu_model = guestfs_int_get_cpu_model (has_kvm && !force_tcg); > > > diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c > > > index 6c0cfe937..a56afbdd4 100644 > > > --- a/lib/launch-libvirt.c > > > +++ b/lib/launch-libvirt.c > > > @@ -773,6 +773,7 @@ parse_capabilities (guestfs_h *g, const char *capabilities_xml, > > > xmlAttrPtr attr; > > > size_t seen_qemu, seen_kvm; > > > int force_tcg; > > > + int force_kvm; > > > > > > doc = xmlReadMemory (capabilities_xml, strlen (capabilities_xml), > > > NULL, NULL, XML_PARSE_NONET); > > > @@ -820,11 +821,15 @@ parse_capabilities (guestfs_h *g, const char *capabilities_xml, > > > } > > > } > > > > > > + force_kvm = guestfs_int_get_backend_setting_bool (g, "force_kvm"); > > > + if (force_kvm == -1) > > > + return -1; > > > + > > > /* This was RHBZ#886915: in that case the default libvirt URI > > > * pointed to a Xen hypervisor, and so could not create the > > > * appliance VM. > > > */ > > > - if (!seen_qemu && !seen_kvm) { > > > + if ((!seen_qemu || force_kvm) && !seen_kvm) { > > If force_kvm is set to true, we enter this if "!seen_kvm" - and fail > > > > CLEANUP_FREE char *backend = guestfs_get_backend (g); > > > > > > error (g, > > > @@ -846,6 +851,11 @@ parse_capabilities (guestfs_h *g, const char *capabilities_xml, > > > if (force_tcg == -1) > > > return -1; > > > > > > + if (force_kvm && force_tcg) { > > > + error (g, "Both force_kvm and force_tcg backend settings supplied."); > > > + return -1; > > > + } > > > + > > > if (!force_tcg) > > That means that seen_kvm = TRUE here > That means that data->is_kvm = TRUE here > > > > data->is_kvm = seen_kvm; > > > else > > > > Where does this actually set KVM only in the libvirt backend? > > Commented above > > >From what I understand this is later translated to "kvm" under > domain->type in libvirt xml. > > I actually needed this only for direct, but thought that I should add > it to libvirt too since that what was done for force_tcg > > > > > Rich. > > > > -- > > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > > Read my programming and virtualization blog: http://rwmj.wordpress.com > > virt-df lists disk usage of guests without needing to install any > > software inside the virtual machine. Supports Linux and Windows. > > http://people.redhat.com/~rjones/virt-df/ > > -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v