[Libguestfs] [p2v PATCH v2 0/6] restrict vCPU topology to (a) fully populated physical, or (b) 1 * N * 1

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 <lersek(a)redhat.com&gt; + Acked-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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 <lersek(a)redhat.com&gt; + Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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 <lersek(a)redhat.com&gt; + Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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 <lersek(a)redhat.com&gt; + Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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 <lersek(a)redhat.com&gt; + Acked-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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 <lersek(a)redhat.com&gt; + Acked-by: Richard W.M. Jones <rjones(a)redhat.com&gt; + 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