11:26 a.m.
I have manually audited all commits that have converted existing
code to use the new helpers (full list below) and have convinced
myself that allowReboot is indeed the only one that requires
special handling.
Changes from [RFC]:
* introduce a separate helper for the one scenario where we need
to accept 'default' as input instead of going for an approach
that would require updating most callers;
* throw in a bunch of extra fixes and improvements.
[RFC] https://listman.redhat.com/archives/libvir-list/2022-March/229506.html
Commits that performed the conversion in a way that accurately
preserved the existing behavior:
14c803cb8276c564d242d4aec8f7d1f64da30321
deac783c86009e0e828746c8c49de70f656aeb85
e1e2e7ec46191a83eadf32be8a87901b01488a6d
cea39e95291517ff2a8ae515573bad587a7dad2d
388cdd11f3c746690edc8e0f71289872c6180c60
dfff3db7763f55b0157f22d816d84c71bbc2dcad
793e71ee76acb49b215e31ee89e0c9cb52322811
92204134806ba4c41ba6cbc20ad2408015e7f3d5
c348da7c4c138c108695c5f309f62e87d0101eda
45abc1a5db60213bf94b5fb780dc6549ddd9103c
45a61cbf68a2459202b7eda4a01a56bfcb6be048
be63e8703c7354bdf0c767a771223b33ab2ad4f2
b13f801d6ff6f9fe5e58aa48fc31bd25ad0b072b
b45ba35e350f2a62c6b06a637d707029bc99e559
fa48004af5ba58cfab38cd8ab5092719a5978509
fcc563a29bd91bd2a45b8f242eccaf37ab75b5b3
97cdb5be1ea84b5224f6353b425ea2503763df07
754a7f6c942268b2b604de072a3391ea4df91e57
b975a8a755b192f5980a14c46e745a92d3cdd3be
e2a38216d2ff6dac8bc927603fdec1ba887b2da7
cd4c756fd5eece6b1d7723a407ead0ed1bf8a298
e663717cb13e49f4da21280bc2f455ddfeee782c
cacde05ad3e8428ad0e16afcfdb715cb4b3d3165
567efa85c2194e45ae943c01c9dffe5b44e81c96
54fb0b9e95c398e8ba09bc7cdceca3f588f910b8
3b7bc307d5fd8e7b9c618a6454d32d59b83277d2
2beae8273b932505888dec35ae97bd8d74893272
0eb42087c7907f43c114cb57b5ff2cf2a52dfea4
b683978f1f4609e7a099ff8b36d7cac25e84cf7c
a85d553d7608bf4c9ffe74546d32afe0275d7c69
86cfd4d4e8de1834607e977318eee4e0c6a9a565
593140dabd66f01aa0d606984ff684f7cb9c1eb1
956373230631929dfa9a36814a283666cb290ee0
0f8fd4548295e3a1516939d7f3bb912a8d7e4713
7ae08ef3a230978510e2722fc56b61ebbae9c6b7
Commits that altered the existing behavior but did so safely, in
that they stopped accepting 'default' as a valid value for a
property that was defined in the schema as only taking either 'yes'
or 'no' and for which libvirt itself never produced the 'default'
value:
9086ae4facfb91436c1d9e7daec9285156cb4eb8
931afa7d99b8ff6eb18a6aa402ed64b789197d30
38dc25989c50dad1f6f64aa038de8c2d1c008734
38d76cde5e90cbb59cd8c726f35646be1740c685
ee387289dd6c993bd2bcfdebcbad86a51fe36da8
550981ce9713ef82df49116c336a9f4bd0eedef4
b127e50290383dc26e8714ba866acc9f501d7af6
54635ea592859d110e87d19565ffabe24d2f0e2e
232c01ec4f650523ab5ff84cf38c4d8b55763052
bb94b3d28db909d43d83b3f2ab73aa3f881b5c95
3681a5393313eba7bfa8e8f763116efb9961c686
Commits that altered the existing behavior in a way that required
some fixing afterwards:
0fe2d8dd335054fae38b46bbbac58a4662e1a1d0
Andrea Bolognani (9):
conf: Don't pass PROP_NONZERO to virXMLPropTristateBool()
conf: Use virTristateBoolToBool() more
qemu: Format <allowReboot> conditionally
util: Introduce virXMLPropTristateBoolAllowDefault()
qemu: Accept <allowReboot value='default'/>
qemu: Don't ignore XMLParseAllowReboot() errors
conf: Restore error checking in VideoAccelDefParseXML()
conf: Format managed property of hostdev-pci ports correctly
network: Convert managed property of hostdev-pci ports correctly
src/conf/domain_conf.c | 42 +++++++++++--------
src/conf/interface_conf.c | 2 +-
src/conf/virnetworkportdef.c | 7 +++-
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 2 +-
src/qemu/qemu_domain.c | 19 +++++----
src/util/virxml.c | 18 ++++++++
src/util/virxml.h | 7 ++++
.../plug-hostdev-pci-unmanaged.xml | 12 ++++++
tests/virnetworkportxml2xmltest.c | 1 +
10 files changed, 83 insertions(+), 28 deletions(-)
create mode 100644 tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
--
2.35.1
11:26 a.m.
New subject: [PATCH 1/9] conf: Don't pass PROP_NONZERO to virXMLPropTristateBool()
It's the default behavior.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 6 +++---
src/conf/interface_conf.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 731139f80f..1186ec8417 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7862,7 +7862,7 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
seclabel->type == VIR_DOMAIN_SECLABEL_NONE)
seclabel->relabel = false;
- if (virXMLPropTristateBool(ctxt->node, "relabel", VIR_XML_PROP_NONZERO,
&relabel) < 0)
+ if (virXMLPropTristateBool(ctxt->node, "relabel", VIR_XML_PROP_NONE,
&relabel) < 0)
return NULL;
virTristateBoolToBool(relabel, &seclabel->relabel);
@@ -8079,7 +8079,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef
***seclabels_rtn,
seclabels[i]->model = g_steal_pointer(&model);
}
- if (virXMLPropTristateBool(list[i], "relabel", VIR_XML_PROP_NONZERO,
&t) < 0)
+ if (virXMLPropTristateBool(list[i], "relabel", VIR_XML_PROP_NONE,
&t) < 0)
goto error;
seclabels[i]->relabel = true;
@@ -8088,7 +8088,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef
***seclabels_rtn,
/* labelskip is only parsed on live images */
seclabels[i]->labelskip = false;
if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
- if (virXMLPropTristateBool(list[i], "labelskip",
VIR_XML_PROP_NONZERO, &t) < 0)
+ if (virXMLPropTristateBool(list[i], "labelskip", VIR_XML_PROP_NONE,
&t) < 0)
goto error;
virTristateBoolToBool(t, &seclabels[i]->labelskip);
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 424650489c..ba5dc9961a 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -233,7 +233,7 @@ virInterfaceDefParseDhcp(virInterfaceProtocolDef *def,
def->dhcp = 1;
def->peerdns = -1;
- if (virXMLPropTristateBool(dhcp, "peerdns", VIR_XML_PROP_NONZERO,
&peerdns) < 0)
+ if (virXMLPropTristateBool(dhcp, "peerdns", VIR_XML_PROP_NONE,
&peerdns) < 0)
return -1;
if (peerdns != VIR_TRISTATE_BOOL_ABSENT) {
--
2.35.1
11:26 a.m.
New subject: [PATCH 2/9] conf: Use virTristateBoolToBool() more
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1186ec8417..68191f42bd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6856,7 +6856,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
if (virXMLPropTristateBool(node, "autoAddress", VIR_XML_PROP_NONE,
&autoAddress) < 0)
return -1;
- usbsrc->autoAddress = autoAddress == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(autoAddress, &usbsrc->autoAddress);
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized */
@@ -12498,7 +12498,7 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDef *def,
&fullscreen) < 0)
return -1;
- def->data.sdl.fullscreen = fullscreen == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(fullscreen, &def->data.sdl.fullscreen);
def->data.sdl.xauth = virXMLPropString(node, "xauth");
def->data.sdl.display = virXMLPropString(node, "display");
@@ -12567,7 +12567,7 @@ virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDef *def,
&fullscreen) < 0)
return -1;
- def->data.desktop.fullscreen = fullscreen == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(fullscreen, &def->data.desktop.fullscreen);
def->data.desktop.display = virXMLPropString(node, "display");
return 0;
@@ -12603,7 +12603,7 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,
if (virXMLPropTristateBool(node, "autoport", VIR_XML_PROP_NONE,
&autoport) < 0)
return -1;
- def->data.spice.autoport = autoport == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(autoport, &def->data.spice.autoport);
def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY;
if (virXMLPropEnum(node, "defaultMode",
@@ -14401,7 +14401,7 @@ virDomainRedirFilterUSBDevDefParseXML(xmlNodePtr node)
if (virXMLPropTristateBool(node, "allow", VIR_XML_PROP_REQUIRED,
&allow) < 0)
return NULL;
- def->allow = allow == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(allow, &def->allow);
return g_steal_pointer(&def);
}
@@ -18071,7 +18071,7 @@ virDomainVcpuParse(virDomainDef *def,
VIR_XML_PROP_REQUIRED, &state) < 0)
return -1;
- vcpu->online = state == VIR_TRISTATE_BOOL_YES;
+ virTristateBoolToBool(state, &vcpu->online);
if (virXMLPropTristateBool(nodes[i], "hotpluggable",
VIR_XML_PROP_NONE,
--
2.35.1
11:26 a.m.
New subject: [PATCH 3/9] qemu: Format <allowReboot> conditionally
If the value is VIR_TRISTATE_BOOL_ABSENT we should just omit
the element entirely.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a587ebe86d..5fe05cc09a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2205,9 +2205,10 @@ void
qemuDomainObjPrivateXMLFormatAllowReboot(virBuffer *buf,
virTristateBool allowReboot)
{
- virBufferAsprintf(buf, "<allowReboot value='%s'/>\n",
- virTristateBoolTypeToString(allowReboot));
-
+ if (allowReboot) {
+ virBufferAsprintf(buf, "<allowReboot value='%s'/>\n",
+ virTristateBoolTypeToString(allowReboot));
+ }
}
--
2.35.1
11:26 a.m.
New subject: [PATCH 4/9] util: Introduce virXMLPropTristateBoolAllowDefault()
We need it for a single scenario in which prop='default' has to
be treated as valid input.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virxml.c | 18 ++++++++++++++++++
src/util/virxml.h | 7 +++++++
3 files changed, 26 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8a3e5f7f7c..a9d7ed972f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3647,6 +3647,7 @@ virXMLPropEnumDefault;
virXMLPropInt;
virXMLPropString;
virXMLPropTristateBool;
+virXMLPropTristateBoolAllowDefault;
virXMLPropTristateSwitch;
virXMLPropUInt;
virXMLPropULongLong;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 8ff59e7cda..e9ccb54c41 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -552,6 +552,24 @@ virXMLPropTristateBool(xmlNodePtr node,
}
+/* Same as virXMLPropTristateBoolAllowDefault, but will accept the
+ * value 'default' and convert it to VIR_TRISTATE_BOOL_ABSENT instead
+ * of rejecting it with an error. Should only be used for backwards
+ * compatibility reasons, and specifically to parse XML files where a
+ * property having value VIR_TRISTATE_BOOL_ABSENT has historically
+ * resulted in it being formatted with value 'default' instead of
+ * being omitted entirely */
+int
+virXMLPropTristateBoolAllowDefault(xmlNodePtr node,
+ const char* name,
+ virXMLPropFlags flags,
+ virTristateBool *result)
+{
+ return virXMLPropEnumInternal(node, name, virTristateBoolTypeFromString,
+ flags, result, VIR_TRISTATE_BOOL_ABSENT);
+}
+
+
/**
* virXMLPropTristateSwitch:
* @node: XML dom node pointer
diff --git a/src/util/virxml.h b/src/util/virxml.h
index c39eae6282..05016db577 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -103,6 +103,13 @@ virXMLPropTristateBool(xmlNodePtr node,
virTristateBool *result)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+int
+virXMLPropTristateBoolAllowDefault(xmlNodePtr node,
+ const char *name,
+ virXMLPropFlags flags,
+ virTristateBool *result)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+
int
virXMLPropTristateSwitch(xmlNodePtr node,
const char *name,
--
2.35.1
11:26 a.m.
New subject: [PATCH 5/9] qemu: Accept <allowReboot value='default'/>
Up until a few commits ago, libvirt produced this XML and so
we need to be able to read it back to prevent a bunch of
error : virXMLPropEnumInternal:516 : XML error: Invalid value
for attribute 'value' in element 'allowReboot': 'default'
messages from being logged on daemon upgrade when there are
running guests.
Fixes: 0fe2d8dd335054fae38b46bbbac58a4662e1a1d0
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5fe05cc09a..88d9499bd0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2859,9 +2859,12 @@ qemuDomainObjPrivateXMLParseAllowReboot(xmlXPathContextPtr ctxt,
{
xmlNodePtr node = virXPathNode("./allowReboot", ctxt);
- return virXMLPropTristateBool(node, "value",
- VIR_XML_PROP_NONE,
- allowReboot);
+ /* Allow value='default' as the input here, because old versions
+ * of libvirt produced that output and we need to be able to read
+ * it back to correctly handle running guests on daemon upgrade */
+ return virXMLPropTristateBoolAllowDefault(node, "value",
+ VIR_XML_PROP_NONE,
+ allowReboot);
}
--
2.35.1
11:26 a.m.
New subject: [PATCH 6/9] qemu: Don't ignore XMLParseAllowReboot() errors
The function may fail and report an error, in which case we
should not just continue as if nothing happened.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 88d9499bd0..d2bca4dd06 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3106,7 +3106,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
priv->chardevStdioLogd = virXPathBoolean("boolean(./chardevStdioLogd)",
ctxt) == 1;
- qemuDomainObjPrivateXMLParseAllowReboot(ctxt, &priv->allowReboot);
+ if (qemuDomainObjPrivateXMLParseAllowReboot(ctxt, &priv->allowReboot) < 0)
+ goto error;
qemuDomainObjPrivateXMLParsePR(ctxt, &priv->prDaemonRunning);
--
2.35.1
11:26 a.m.
New subject: [PATCH 7/9] conf: Restore error checking in VideoAccelDefParseXML()
The check that ensures that at least one property among accel3d,
accel2d and rendernode has been provided by the user had been
lost during the conversion to virXMLPropTristateBool().
Fixes: 0fe2d8dd335054fae38b46bbbac58a4662e1a1d0
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 68191f42bd..bd98317f4d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14019,23 +14019,31 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
{
g_autofree virDomainVideoAccelDef *def = NULL;
g_autofree char *rendernode = NULL;
+ virTristateBool accel3d;
+ virTristateBool accel2d;
rendernode = virXMLPropString(node, "rendernode");
-
- def = g_new0(virDomainVideoAccelDef, 1);
-
if (virXMLPropTristateBool(node, "accel3d",
- VIR_XML_PROP_NONE,
- &def->accel3d) < 0)
+ VIR_XML_PROP_NONE, &accel3d) < 0)
return NULL;
-
if (virXMLPropTristateBool(node, "accel2d",
- VIR_XML_PROP_NONE,
- &def->accel2d) < 0)
+ VIR_XML_PROP_NONE, &accel2d) < 0)
return NULL;
+ if (!rendernode &&
+ accel3d == VIR_TRISTATE_BOOL_ABSENT &&
+ accel2d == VIR_TRISTATE_BOOL_ABSENT) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("missing values for acceleration"));
+ return NULL;
+ }
+
+ def = g_new0(virDomainVideoAccelDef, 1);
+
if (rendernode)
def->rendernode = virFileSanitizePath(rendernode);
+ def->accel3d = accel3d;
+ def->accel2d = accel2d;
return g_steal_pointer(&def);
}
--
2.35.1
11:26 a.m.
New subject: [PATCH 8/9] conf: Format managed property of hostdev-pci ports correctly
The property is parsed using virTristateBoolTypeFromString() but
formatted as if it was a regular bool, which results in the
following incorrect conversion:
BOOL_ABSENT -> managed='no'
BOOL_YES -> managed='yes'
BOOL_NO -> managed='yes'
Use the virTristateBoolTypeToString() helper to ensure the
setting can survive a roundtrip conversion.
Fixes: 4b4a981d60d3372fb73bb3546ab1052844b2e3d5
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/virnetworkportdef.c | 7 +++++--
.../plug-hostdev-pci-unmanaged.xml | 12 ++++++++++++
tests/virnetworkportxml2xmltest.c | 1 +
3 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c
index fcd9e55a55..aa35374fb0 100644
--- a/src/conf/virnetworkportdef.c
+++ b/src/conf/virnetworkportdef.c
@@ -384,8 +384,11 @@ virNetworkPortDefFormatBuf(virBuffer *buf,
break;
case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
- virBufferAsprintf(buf, " managed='%s'>\n",
- def->plug.hostdevpci.managed ? "yes" :
"no");
+ if (def->plug.hostdevpci.managed) {
+ virBufferAsprintf(buf, " managed='%s'",
+
virTristateBoolTypeToString(def->plug.hostdevpci.managed));
+ }
+ virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
if (def->plug.hostdevpci.driver)
virBufferEscapeString(buf, "<driver
name='%s'/>\n",
diff --git a/tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
b/tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
new file mode 100644
index 0000000000..da5f568031
--- /dev/null
+++ b/tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
@@ -0,0 +1,12 @@
+<networkport>
+ <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
+ <owner>
+ <name>memtest</name>
+ <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
+ </owner>
+ <mac address='52:54:00:7b:35:93'/>
+ <plug type='hostdev-pci' managed='no'>
+ <driver name='vfio'/>
+ <address domain='0x0001' bus='0x02' slot='0x03'
function='0x4'/>
+ </plug>
+</networkport>
diff --git a/tests/virnetworkportxml2xmltest.c b/tests/virnetworkportxml2xmltest.c
index 093d855633..176f52d675 100644
--- a/tests/virnetworkportxml2xmltest.c
+++ b/tests/virnetworkportxml2xmltest.c
@@ -85,6 +85,7 @@ mymain(void)
DO_TEST("plug-bridge-mactbl");
DO_TEST("plug-direct");
DO_TEST("plug-hostdev-pci");
+ DO_TEST("plug-hostdev-pci-unmanaged");
DO_TEST("plug-network");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--
2.35.1
11:26 a.m.
New subject: [PATCH 9/9] network: Convert managed property of hostdev-pci ports correctly
virNetworkForwardDef.managed is a bool but
virNetworkPortDef.hostdevpci.managed is a virTristateBool, which
means that the current code performs the following incorrect
conversion:
false -> BOOL_ABSENT
true -> BOOL_YES
Using the virTristateBoolFromBool() helper solves the issue.
Fixes: 6cb0ec48bd95c95489a987e05a88e8bcf1f9109c
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/network/bridge_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 67c5a111b3..e7049ba384 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3991,7 +3991,7 @@ networkAllocatePort(virNetworkObj *obj,
}
port->plug.hostdevpci.addr = dev->device.pci;
port->plug.hostdevpci.driver = netdef->forward.driverName;
- port->plug.hostdevpci.managed = netdef->forward.managed;
+ port->plug.hostdevpci.managed =
virTristateBoolFromBool(netdef->forward.managed);
if (port->virtPortProfile) {
/* make sure type is supported for hostdev connections */
--
2.35.1
3:56 a.m.
On 3/25/22 17:26, Andrea Bolognani wrote:
I have manually audited all commits that have converted existing
code to use the new helpers (full list below) and have convinced
myself that allowReboot is indeed the only one that requires
special handling.
Changes from [RFC]:
* introduce a separate helper for the one scenario where we need
to accept 'default' as input instead of going for an approach
that would require updating most callers;
* throw in a bunch of extra fixes and improvements.
[RFC] https://listman.redhat.com/archives/libvir-list/2022-March/229506.html
Commits that performed the conversion in a way that accurately
preserved the existing behavior:
14c803cb8276c564d242d4aec8f7d1f64da30321
deac783c86009e0e828746c8c49de70f656aeb85
e1e2e7ec46191a83eadf32be8a87901b01488a6d
cea39e95291517ff2a8ae515573bad587a7dad2d
388cdd11f3c746690edc8e0f71289872c6180c60
dfff3db7763f55b0157f22d816d84c71bbc2dcad
793e71ee76acb49b215e31ee89e0c9cb52322811
92204134806ba4c41ba6cbc20ad2408015e7f3d5
c348da7c4c138c108695c5f309f62e87d0101eda
45abc1a5db60213bf94b5fb780dc6549ddd9103c
45a61cbf68a2459202b7eda4a01a56bfcb6be048
be63e8703c7354bdf0c767a771223b33ab2ad4f2
b13f801d6ff6f9fe5e58aa48fc31bd25ad0b072b
b45ba35e350f2a62c6b06a637d707029bc99e559
fa48004af5ba58cfab38cd8ab5092719a5978509
fcc563a29bd91bd2a45b8f242eccaf37ab75b5b3
97cdb5be1ea84b5224f6353b425ea2503763df07
754a7f6c942268b2b604de072a3391ea4df91e57
b975a8a755b192f5980a14c46e745a92d3cdd3be
e2a38216d2ff6dac8bc927603fdec1ba887b2da7
cd4c756fd5eece6b1d7723a407ead0ed1bf8a298
e663717cb13e49f4da21280bc2f455ddfeee782c
cacde05ad3e8428ad0e16afcfdb715cb4b3d3165
567efa85c2194e45ae943c01c9dffe5b44e81c96
54fb0b9e95c398e8ba09bc7cdceca3f588f910b8
3b7bc307d5fd8e7b9c618a6454d32d59b83277d2
2beae8273b932505888dec35ae97bd8d74893272
0eb42087c7907f43c114cb57b5ff2cf2a52dfea4
b683978f1f4609e7a099ff8b36d7cac25e84cf7c
a85d553d7608bf4c9ffe74546d32afe0275d7c69
86cfd4d4e8de1834607e977318eee4e0c6a9a565
593140dabd66f01aa0d606984ff684f7cb9c1eb1
956373230631929dfa9a36814a283666cb290ee0
0f8fd4548295e3a1516939d7f3bb912a8d7e4713
7ae08ef3a230978510e2722fc56b61ebbae9c6b7
Commits that altered the existing behavior but did so safely, in
that they stopped accepting 'default' as a valid value for a
property that was defined in the schema as only taking either 'yes'
or 'no' and for which libvirt itself never produced the 'default'
value:
9086ae4facfb91436c1d9e7daec9285156cb4eb8
931afa7d99b8ff6eb18a6aa402ed64b789197d30
38dc25989c50dad1f6f64aa038de8c2d1c008734
38d76cde5e90cbb59cd8c726f35646be1740c685
ee387289dd6c993bd2bcfdebcbad86a51fe36da8
550981ce9713ef82df49116c336a9f4bd0eedef4
b127e50290383dc26e8714ba866acc9f501d7af6
54635ea592859d110e87d19565ffabe24d2f0e2e
232c01ec4f650523ab5ff84cf38c4d8b55763052
bb94b3d28db909d43d83b3f2ab73aa3f881b5c95
3681a5393313eba7bfa8e8f763116efb9961c686
Commits that altered the existing behavior in a way that required
some fixing afterwards:
0fe2d8dd335054fae38b46bbbac58a4662e1a1d0
Andrea Bolognani (9):
conf: Don't pass PROP_NONZERO to virXMLPropTristateBool()
conf: Use virTristateBoolToBool() more
qemu: Format <allowReboot> conditionally
util: Introduce virXMLPropTristateBoolAllowDefault()
qemu: Accept <allowReboot value='default'/>
qemu: Don't ignore XMLParseAllowReboot() errors
conf: Restore error checking in VideoAccelDefParseXML()
conf: Format managed property of hostdev-pci ports correctly
network: Convert managed property of hostdev-pci ports correctly
src/conf/domain_conf.c | 42 +++++++++++--------
src/conf/interface_conf.c | 2 +-
src/conf/virnetworkportdef.c | 7 +++-
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 2 +-
src/qemu/qemu_domain.c | 19 +++++----
src/util/virxml.c | 18 ++++++++
src/util/virxml.h | 7 ++++
.../plug-hostdev-pci-unmanaged.xml | 12 ++++++
tests/virnetworkportxml2xmltest.c | 1 +
10 files changed, 83 insertions(+), 28 deletions(-)
create mode 100644 tests/virnetworkportxml2xmldata/plug-hostdev-pci-unmanaged.xml
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
957
days inactive
968
days old
10 comments
2 participants
participants (2)
-
Andrea Bolognani -
Michal Prívozník