Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 v1: https://listman.redhat.com/archives/libguestfs/2022-September/029806.html
Please see the Notes section on each patch for the updates in this version (addressing v1 feedback). Patch #5 is a candidate for dropping in particular. I'm also including a range-diff between v1 and v2, below. > 1: 4ec0fa83a1a2 ! 1: bdbd76659e43 gui: check VCPU & memory limits upon > showing the conversion dialog > @@ -17,6 +17,9 @@ > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Acked-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + - pick up Rich's ACK > > diff --git a/gui.c b/gui.c > --- a/gui.c > 2: 9f7005fecaf4 ! 2: 0924f02a260b restrict vCPU topology to (a) fully > populated physical, or (b) 1 * N * 1 > @@ -113,6 +113,16 @@ > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + > + - pick up Rich's R-b > + > + - rename dense_topo to phys_topo [Rich] > + > + - take Rich's suggested wording for the manual > + > + - rewrap the additions to the manual > > diff --git a/generate-p2v-config.pl b/generate-p2v-config.pl > --- a/generate-p2v-config.pl > @@ -125,7 +135,7 @@ > + ConfigSection->new( > + name => 'vcpu', > + elements => [ > -+ ConfigBool->new(name => 'dense_topo'), > ++ ConfigBool->new(name => 'phys_topo'), > + ConfigInt->new(name => 'cores', value => 0), > + ], > + ), > @@ -146,23 +156,23 @@ > use a randomly generated name.", > ), > - "p2v.vcpus" => manual_entry->new( > -+ "p2v.vcpu.dense_topo" => manual_entry->new( > ++ "p2v.vcpu.phys_topo" => manual_entry->new( > + shortopt => "", # ignored for booleans > + description => " > -+Copy the physical machine's CPU topology, densely populated, to the > -+guest. Disabled by default. If disabled, the C<p2v.vcpu.cores> setting > -+takes effect.", > ++Copy the physical machine's complete CPU topology (sockets, cores and > ++threads) to the guest. Disabled by default. If disabled, the > ++C<p2v.vcpu.cores> setting takes effect.", > + ), > + "p2v.vcpu.cores" => manual_entry->new( > shortopt => "N", > description => " > -The number of virtual CPUs to give to the guest. The default is to > -use the same as the number of physical CPUs.", > -+This setting is ignored if C<p2v.vcpu.dense_topo> is enabled. > -+Otherwise, it specifies the flat number of vCPU cores to give to the > -+guest (placing all of those cores into a single socket, and exposing one > -+thread per core). The default value is the number of online logical > -+processors on the physical machine.", > ++This setting is ignored if C<p2v.vcpu.phys_topo> is enabled. Otherwise, > ++it specifies the flat number of vCPU cores to give to the guest (placing > ++all of those cores into a single socket, and exposing one thread per > ++core). The default value is the number of online logical processors on > ++the physical machine.", > ), > "p2v.memory" => manual_entry->new( > shortopt => "n(M|G)", > @@ -297,7 +307,7 @@ > } > config->guestname = strdup (hostname); > > -+ config->vcpu.dense_topo = false; > ++ config->vcpu.phys_topo = false; > + > /* Defaults for #vcpus and memory are taken from the physical > machine. */ > i = sysconf (_SC_NPROCESSORS_ONLN); > @@ -357,7 +367,7 @@ > - } end_element (); > - } > - } end_element (); > -+ if (config->vcpu.dense_topo) > ++ if (config->vcpu.phys_topo) > + get_cpu_topology (&topo); > + else { > + topo.sockets = 1; > @@ -407,7 +417,7 @@ > grep "^auth\.sudo.*false" $out > grep "^guestname.*test" $out > -grep "^vcpus.*4" $out > -+grep "^vcpu.dense_topo.*false" $out > ++grep "^vcpu.phys_topo.*false" $out > +grep "^vcpu.cores.*4" $out > grep "^memory.*"$((1024*1024*1024)) $out > grep "^disks.*sda sdb sdc" $out > 3: 3e873b661eed ! 3: 8c37949d1ea2 gui: set row count from a running > variable when populating tables > @@ -14,8 +14,8 @@ > > (This patch is easiest to review with "git show --word-diff=color".) > > - Don't do the same simplification for colums, as we're going to > introduce a > - multi-column widget next. > + Don't do the same simplification for columns, as we're going to > introduce > + a multi-column widget next. > > Note that one definition of table_attach() now evaluates "top" twice. > Preventing that would be a mess: we could be tempted to introduce a > do { > @@ -28,6 +28,12 @@ > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + > + - s/colums/columns/ in the commit message [Rich] > + > + - pick up Rich's R-b > > diff --git a/gui-gtk3-compat.h b/gui-gtk3-compat.h > --- a/gui-gtk3-compat.h > 4: 1fcf2898202f ! 4: 6e3a17d86f89 gui: offer copying the vCPU topology from > the fully populated physical one > @@ -25,6 +25,12 @@ > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + > + - pick up Rich's R-b > + > + - s/dense_topo/phys_topo/ everywhere [Rich] > > diff --git a/gui.c b/gui.c > --- a/gui.c > @@ -51,7 +57,7 @@ > +static void vcpu_topo_toggled (GtkWidget *w, gpointer data); > static void vcpus_or_memory_check_callback (GtkWidget *w, gpointer > data); > static void notify_ui_callback (int type, const char *data); > -+static bool get_dense_topo_from_conv_dlg (void); > ++static bool get_phys_topo_from_conv_dlg (void); > static int get_vcpus_from_conv_dlg (void); > static uint64_t get_memory_from_conv_dlg (void); > > @@ -72,7 +78,7 @@ > + vcpu_topo = gtk_check_button_new_with_mnemonic ( > + _("Copy fully populated _pCPU topology")); > + gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (vcpu_topo), > -+ config->vcpu.dense_topo); > ++ config->vcpu.phys_topo); > + table_attach (target_tbl, vcpu_topo, 0, 2, row, GTK_FILL, GTK_FILL, > 1, 1); > + > row++; > @@ -110,12 +116,12 @@ > +static void > +vcpu_topo_toggled (GtkWidget *w, gpointer data) > +{ > -+ bool dense_topo; > ++ bool phys_topo; > + unsigned vcpus; > + char vcpus_str[64]; > + > -+ dense_topo = get_dense_topo_from_conv_dlg (); > -+ if (dense_topo) { > ++ phys_topo = get_phys_topo_from_conv_dlg (); > ++ if (phys_topo) { > + struct cpu_topo topo; > + > + get_cpu_topology (&topo); > @@ -126,7 +132,7 @@ > + > + snprintf (vcpus_str, sizeof vcpus_str, "%u", vcpus); > + gtk_entry_set_text (GTK_ENTRY (vcpus_entry), vcpus_str); > -+ gtk_widget_set_sensitive (vcpus_entry, !dense_topo); > ++ gtk_widget_set_sensitive (vcpus_entry, !phys_topo); > +} > + > /** > @@ -137,7 +143,7 @@ > } > > +static bool > -+get_dense_topo_from_conv_dlg (void) > ++get_phys_topo_from_conv_dlg (void) > +{ > + return gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (vcpu_topo)); > +} > @@ -149,7 +155,7 @@ > return; > } > > -+ config->vcpu.dense_topo = get_dense_topo_from_conv_dlg (); > ++ config->vcpu.phys_topo = get_phys_topo_from_conv_dlg (); > config->vcpu.cores = get_vcpus_from_conv_dlg (); > config->memory = get_memory_from_conv_dlg (); > > 5: a5ce2ce0843c ! 5: 3e87bcfec5ab copy fully populated vCPU topology by > default > @@ -6,6 +6,15 @@ > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Acked-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + > + - pick up Rich's A-b > + > + - resolve rebase conflicts due to dense_topo -> phys_topo renaming > + > + - we can drop this patch, per Daniel's comment > + > <https://listman.redhat.com/archives/libguestfs/2022-September/029841.html> > > diff --git a/generate-p2v-config.pl b/generate-p2v-config.pl > --- a/generate-p2v-config.pl > @@ -13,10 +22,10 @@ > @@ > shortopt => "", # ignored for booleans > description => " > - Copy the physical machine's CPU topology, densely populated, to the > --guest. Disabled by default. If disabled, the C<p2v.vcpu.cores> setting > -+guest. This is the default. If disabled, the C<p2v.vcpu.cores> setting > - takes effect.", > + Copy the physical machine's complete CPU topology (sockets, cores and > +-threads) to the guest. Disabled by default. If disabled, the > ++threads) to the guest. This is the default. If disabled, the > + C<p2v.vcpu.cores> setting takes effect.", > ), > "p2v.vcpu.cores" => manual_entry->new( > > @@ -27,8 +36,8 @@ > } > config->guestname = strdup (hostname); > > -- config->vcpu.dense_topo = false; > -+ config->vcpu.dense_topo = true; > +- config->vcpu.phys_topo = false; > ++ config->vcpu.phys_topo = true; > > /* Defaults for #vcpus and memory are taken from the physical > machine. */ > i = sysconf (_SC_NPROCESSORS_ONLN); > @@ -41,7 +50,7 @@ > > # The Linux kernel command line. > -$VG virt-p2v --cmdline='p2v.server=localhost p2v.port=123 > p2v.username=user p2v.password=secret p2v.skip_test_connection p2v.name=test > p2v.vcpu.cores=4 p2v.memory=1G p2v.disks=sda,sdb,sdc p2v.removable=sdd > p2v.interfaces=eth0,eth1 p2v.o=local p2v.oa=sparse p2v.oc=qemu:///session > p2v.of=raw p2v.os=/var/tmp p2v.network=em1:wired,other > p2v.dump_config_and_exit' > $out > -+$VG virt-p2v --cmdline='p2v.server=localhost p2v.port=123 > p2v.username=user p2v.password=secret p2v.skip_test_connection p2v.name=test > p2v.vcpu.dense_topo=false p2v.vcpu.cores=4 p2v.memory=1G > p2v.disks=sda,sdb,sdc p2v.removable=sdd p2v.interfaces=eth0,eth1 p2v.o=local > p2v.oa=sparse p2v.oc=qemu:///session p2v.of=raw p2v.os=/var/tmp > p2v.network=em1:wired,other p2v.dump_config_and_exit' > $out > ++$VG virt-p2v --cmdline='p2v.server=localhost p2v.port=123 > p2v.username=user p2v.password=secret p2v.skip_test_connection p2v.name=test > p2v.vcpu.phys_topo=false p2v.vcpu.cores=4 p2v.memory=1G p2v.disks=sda,sdb,sdc > p2v.removable=sdd p2v.interfaces=eth0,eth1 p2v.o=local p2v.oa=sparse > p2v.oc=qemu:///session p2v.of=raw p2v.os=/var/tmp p2v.network=em1:wired,other > p2v.dump_config_and_exit' > $out > > # For debugging purposes. > cat $out > 6: d9d09c8ad7e6 ! 6: 1ecbd348a4b3 Makefile.am: set vCPU topology to 1*2*2 > in the "p2v in a VM" tests > @@ -2,11 +2,17 @@ > > Makefile.am: set vCPU topology to 1*2*2 in the "p2v in a VM" tests > > - This lets us exercise both states of the "p2v.vcpu.dense_topo" switch > + This lets us exercise both states of the "p2v.vcpu.phys_topo" switch > sensibly via the in-VM GUI. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > + Acked-by: Richard W.M. Jones <rjo...@redhat.com> > + v2: > + > + - pick up Rich's A-b > + > + - s/dense_topo/phys_topo/ in the commit message [Rich] > > diff --git a/Makefile.am b/Makefile.am > --- a/Makefile.am Thanks, Laszlo Laszlo Ersek (6): gui: check VCPU & memory limits upon showing the conversion dialog restrict vCPU topology to (a) fully populated physical, or (b) 1 * N * 1 gui: set row count from a running variable when populating tables gui: offer copying the vCPU topology from the fully populated physical one copy fully populated vCPU topology by default Makefile.am: set vCPU topology to 1*2*2 in the "p2v in a VM" tests Makefile.am | 2 + generate-p2v-config.pl | 45 ++++--- gui-gtk3-compat.h | 8 +- p2v.h | 6 + cpuid.c | 39 ++++--- gui.c | 123 ++++++++++++++------ main.c | 8 +- physical-xml.c | 54 ++++----- test-virt-p2v-cmdline.sh | 5 +- 9 files changed, 182 insertions(+), 108 deletions(-) _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
