This doesn't seem very useful at the moment, but it will make
sense once we introduce another HPT-related setting.
The output XML is decoupled from the input XML in preparation
of future changes as well; while doing so, we can shave a few
lines off the latter.
This commit is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/conf/domain_conf.c | 25 ++++++++++++---
src/qemu/qemu_command.c | 32 ++++++++++---------
tests/qemuxml2argvdata/pseries-features.xml | 14 ++------
tests/qemuxml2xmloutdata/pseries-features.xml | 29 ++++++++++++++++-
4 files changed, 68 insertions(+), 32 deletions(-)
mode change 120000 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 355e497002..20b845f02a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27207,14 +27207,31 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virDomainIOAPICTypeToString(def->features[i]));
break;
- case VIR_DOMAIN_FEATURE_HPT:
+ case VIR_DOMAIN_FEATURE_HPT: {
+ bool hasResizing = def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE;
+ char *resizing = NULL;
+
if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
- def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE)
+ !hasResizing) {
break;
+ }
+
+ if (hasResizing) {
+ if (virAsprintf(&resizing, " resizing='%s'",
+ virDomainHPTResizingTypeToString(def->hpt_resizing)) < 0) {
+ goto error;
+ }
+ } else {
+ if (VIR_STRDUP(resizing, "") < 0)
+ goto error;
+ }
- virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
- virDomainHPTResizingTypeToString(def->hpt_resizing));
+ virBufferAsprintf(buf, "<hpt%s/>\n",
+ resizing);
+
+ VIR_FREE(resizing);
break;
+ }
/* coverity[dead_error_begin] */
case VIR_DOMAIN_FEATURE_LAST:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ddbde54a8c..b446a08613 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7173,24 +7173,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
}
if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) {
- const char *str;
- if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("HTP resizing is not supported by this "
- "QEMU binary"));
- goto cleanup;
- }
+ if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) {
+ const char *str;
- str = virDomainHPTResizingTypeToString(def->hpt_resizing);
- if (!str) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Invalid setting for HPT resizing"));
- goto cleanup;
- }
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("HTP resizing is not supported by this "
+ "QEMU binary"));
+ goto cleanup;
+ }
+
+ str = virDomainHPTResizingTypeToString(def->hpt_resizing);
+ if (!str) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Invalid setting for HPT resizing"));
+ goto cleanup;
+ }
- virBufferAsprintf(&buf, ",resize-hpt=%s", str);
+ virBufferAsprintf(&buf, ",resize-hpt=%s", str);
+ }
}
if (cpu && cpu->model &&
diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml
index 5dd0dbd0be..5ef1a744c8 100644
--- a/tests/qemuxml2argvdata/pseries-features.xml
+++ b/tests/qemuxml2argvdata/pseries-features.xml
@@ -2,27 +2,17 @@
<name>guest</name>
<uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid>
<memory unit='KiB'>524288</memory>
- <currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='ppc64' machine='pseries'>hvm</type>
- <boot dev='hd'/>
</os>
<features>
<hpt resizing='required'/>
</features>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-ppc64</emulator>
- <controller type='usb' index='0' model='none'/>
- <controller type='pci' index='0' model='pci-root'>
- <model name='spapr-pci-host-bridge'/>
- <target index='0'/>
- </controller>
+ <controller type='pci' model='pci-root'/>
+ <controller type='usb' model='none'/>
<memballoon model='none'/>
- <panic model='pseries'/>
</devices>
</domain>
diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml
deleted file mode 120000
index 1b01dbace6..0000000000
--- a/tests/qemuxml2xmloutdata/pseries-features.xml
+++ /dev/null
@@ -1 +0,0 @@
-../qemuxml2argvdata/pseries-features.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml
new file mode 100644
index 0000000000..e8ed842fb6
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pseries-features.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid>
+ <memory unit='KiB'>524288</memory>
+ <currentMemory unit='KiB'>524288</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='ppc64' machine='pseries'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <hpt resizing='required'/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-ppc64</emulator>
+ <controller type='pci' index='0' model='pci-root'>
+ <model name='spapr-pci-host-bridge'/>
+ <target index='0'/>
+ </controller>
+ <controller type='usb' index='0' model='none'/>
+ <memballoon model='none'/>
+ <panic model='pseries'/>
+ </devices>
+</domain>
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> This doesn't seem very useful at the moment, but it will make
> sense once we introduce another HPT-related setting.
>
> The output XML is decoupled from the input XML in preparation
> of future changes as well; while doing so, we can shave a few
> lines off the latter.
>
> This commit is best viewed with 'git show -w'.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/conf/domain_conf.c | 25 ++++++++++++---
> src/qemu/qemu_command.c | 32 ++++++++++---------
> tests/qemuxml2argvdata/pseries-features.xml | 14 ++------
> tests/qemuxml2xmloutdata/pseries-features.xml | 29 ++++++++++++++++-
> 4 files changed, 68 insertions(+), 32 deletions(-)
> mode change 120000 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 355e497002..20b845f02a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -27207,14 +27207,31 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> virDomainIOAPICTypeToString(def->features[i]));
> break;
>
> - case VIR_DOMAIN_FEATURE_HPT:
> + case VIR_DOMAIN_FEATURE_HPT: {
> + bool hasResizing = def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE;
> + char *resizing = NULL;
> +
> if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
> - def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE)
> + !hasResizing) {
> break;
> + }
> +
> + if (hasResizing) {
> + if (virAsprintf(&resizing, " resizing='%s'",
> + virDomainHPTResizingTypeToString(def->hpt_resizing)) < 0) {
> + goto error;
> + }
> + } else {
> + if (VIR_STRDUP(resizing, "") < 0)
> + goto error;
> + }
>
> - virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> - virDomainHPTResizingTypeToString(def->hpt_resizing));
> + virBufferAsprintf(buf, "<hpt%s/>\n",
This formulation looks fishy.
> + resizing);
> +
> + VIR_FREE(resizing);
> break;
> + }
>
> /* coverity[dead_error_begin] */
> case VIR_DOMAIN_FEATURE_LAST:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > + if (hasResizing) {
> > + if (virAsprintf(&resizing, " resizing='%s'",
> > + virDomainHPTResizingTypeToStri
> > ng(def->hpt_resizing)) < 0) {
> > + goto error;
> > + }
> > + } else {
> > + if (VIR_STRDUP(resizing, "") < 0)
> > + goto error;
> > + }
> >
> > - virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> > - virDomainHPTResizingTypeToString
> > (def->hpt_resizing));
> > + virBufferAsprintf(buf, "<hpt%s/>\n",
>
> This formulation looks fishy.
I don't love it either, but I've tried a bunch of alternative
approaches and this seemed like the most sane to me.
If you have suggestions on how to improve it, considering that the
end result is what you see after patch 5/6, please do share! :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, May 23, 2018 at 18:50:04 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> > On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > > + if (hasResizing) {
> > > + if (virAsprintf(&resizing, " resizing='%s'",
> > > + virDomainHPTResizingTypeToStri
> > > ng(def->hpt_resizing)) < 0) {
> > > + goto error;
> > > + }
> > > + } else {
> > > + if (VIR_STRDUP(resizing, "") < 0)
> > > + goto error;
> > > + }
> > >
> > > - virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> > > - virDomainHPTResizingTypeToString
> > > (def->hpt_resizing));
> > > + virBufferAsprintf(buf, "<hpt%s/>\n",
> >
> > This formulation looks fishy.
>
> I don't love it either, but I've tried a bunch of alternative
> approaches and this seemed like the most sane to me.
>
> If you have suggestions on how to improve it, considering that the
> end result is what you see after patch 5/6, please do share! :)
virXMLFormatElement automatically closes the tag if the provided
'attrBuf' is empty. Currently it will not work for this particular case
but I think it is worth to add a version which will format the element
even if both buffers are empty.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2018-05-24 at 07:18 +0200, Peter Krempa wrote:
> On Wed, May 23, 2018 at 18:50:04 +0200, Andrea Bolognani wrote:
> > On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> > > On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > > > + if (hasResizing) {
> > > > + if (virAsprintf(&resizing, " resizing='%s'",
> > > > + virDomainHPTResizingTypeToStri
> > > > ng(def->hpt_resizing)) < 0) {
> > > > + goto error;
> > > > + }
> > > > + } else {
> > > > + if (VIR_STRDUP(resizing, "") < 0)
> > > > + goto error;
> > > > + }
> > > >
> > > > - virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
> > > > - virDomainHPTResizingTypeToString
> > > > (def->hpt_resizing));
> > > > + virBufferAsprintf(buf, "<hpt%s/>\n",
> > >
> > > This formulation looks fishy.
Okay, I think I see now why you called it fishy :)
Basically there are three possibilities for how the output will
look like:
<hpt resizing='x'/>
<hpt>
<maxpagesize>x</maxpagesize>
</hpt>
<hpt resizing='x'>
<maxpagesize>x</maxpagesize>
</hpt>
Checks against hasResizing and hasMaxPageSize ensure only the
combinations that make sense result in output being produced, so
<hpt/>
will never happen, but that's not immediately apparent by looking
at the snippet above.
> > I don't love it either, but I've tried a bunch of alternative
> > approaches and this seemed like the most sane to me.
> >
> > If you have suggestions on how to improve it, considering that the
> > end result is what you see after patch 5/6, please do share! :)
>
> virXMLFormatElement automatically closes the tag if the provided
> 'attrBuf' is empty. Currently it will not work for this particular case
> but I think it is worth to add a version which will format the element
> even if both buffers are empty.
I was not aware of that function, thanks for mentioning it!
As explained above, the case where both buffers are empty is not
supposed to produce any output, so the existing function already
does exactly what I need :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.