Re: [libvirt PATCH v2 2/2] esx: implement domainInterfaceAddresses
On 9/14/20 6:59 PM, Pino Toscano wrote: Implement the .domainInterfaceAddresses hypervisor API, although only functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source. Signed-off-by: Pino Toscano --- docs/drvesx.html.in | 4 + src/esx/esx_driver.c | 170 +++ 2 files changed, 174 insertions(+) For both: Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH v2] esx: implement connectListAllNetworks
On 9/14/20 6:31 PM, Pino Toscano wrote: Implement the .connectListAllNetworks networks API in the esx network driver. Signed-off-by: Pino Toscano --- src/esx/esx_network_driver.c | 69 1 file changed, 69 insertions(+) Reviewed-by: Michal Privoznik Michal
[PATCH v1 4/4] NEWS.rst: document the pSeries NVDIMM auto-alignment revival
Signed-off-by: Daniel Henrique Barboza --- NEWS.rst | 12 1 file changed, 12 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 14f098b1f6..9020696f86 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,6 +28,18 @@ v6.8.0 (unreleased) useful for containerised scenarios and can be used in both peer2peer and direct migrations. + * Re-introduce NVDIMM auto-alignment for pSeries Guests + +This feature was removed in libvirt v6.7.0 due to its shortcomings, namely +the lack of consistency between domain XML and actual NVDIMM size the guest +is using, but it came at a too high of a cost - we broke existing guests +that were running in libvirt v.6.6.0 and now had to adjust the size by +hand. The auto-alignment was re-introduced back, allowing existing guests +to keep running as usual. But now, instead of auto-aligning in a transparent +manner, it is also changing the domain XML. This brings a good balance +between consistency of domain XML and guest information, and also relieves +the user of knowing the alignment internals of pSeries NVDIMMs. + * **Bug fixes** -- 2.26.2
[PATCH v1 0/4] revert latest PPC64 NVDIMM changes
Hi, In [1], Daniel pointed out that the changes made by [2] are uncompliant with the Libvirt design of not doing anything that can compromise guests in the wild. This series aims to solve that by reverting the changes made by [2]. It also enhances the auto-alignment of ppc64 NVDIMM in a way that we can have consistency between the sizes reported in the domain XML and what QEMU will see, while not hurting any running guests that are already using the auto-alignment feature. For simplicity, first patch contains the revert of all relevant patches from [2] in a single commit. [1] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html [2] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html Daniel Henrique Barboza (4): qemu: revert latest pSeries NVDIMM design changes conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefParseXML() NEWS.rst: document the pSeries NVDIMM auto-alignment revival NEWS.rst | 12 docs/formatdomain.rst | 6 +- src/conf/domain_conf.c| 71 +++ src/conf/domain_conf.h| 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 19 +++-- src/qemu/qemu_domain.h| 6 +- src/qemu/qemu_hotplug.c | 6 +- src/qemu/qemu_validate.c | 42 ++- ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- .../memory-hotplug-nvdimm-ppc64.xml | 2 +- 12 files changed, 105 insertions(+), 67 deletions(-) -- 2.26.2
[PATCH v1 3/4] domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefParseXML()
The alignment for the pSeries NVDIMM does not depend on runtime
constraints. This means that it can be done in device parse
time, instead of runtime, allowing the domain XML to reflect
what the auto-alignment would do when the domain starts.
This brings consistency between the NVDIMM size reported by the
domain XML and what the guest sees, without impacting existing
guests that are using an unaligned size - they'll work as usual,
but the domain XML will be updated with the actual size of the
NVDIMM.
Signed-off-by: Daniel Henrique Barboza
---
src/conf/domain_conf.c| 35 +++
.../memory-hotplug-nvdimm-ppc64.xml | 2 +-
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 63434b9d3e..0fd16964c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16965,6 +16965,25 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr
xmlopt,
}
VIR_FREE(tmp);
+/* source */
+if ((node = virXPathNode("./source", ctxt)) &&
+virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
+goto error;
+
+/* target */
+if (!(node = virXPathNode("./target", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing element for device"));
+goto error;
+}
+
+if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
+goto error;
+
+/* The UUID calculation for pSeries NVDIMM can be done earlier,
+ * but we'll also need to handle the size alignment, which depends
+ * on virDomainMemoryTargetDefParseXML() already done. Let's do it
+ * all here. */
if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
ARCH_IS_PPC64(dom->os.arch)) {
/* Extract nvdimm uuid or generate a new one */
@@ -16981,23 +17000,11 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr
xmlopt,
"%s", _("malformed uuid element"));
goto error;
}
-}
-
-/* source */
-if ((node = virXPathNode("./source", ctxt)) &&
-virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
-goto error;
-/* target */
-if (!(node = virXPathNode("./target", ctxt))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing element for device"));
-goto error;
+if (virDomainNVDimmAlignSizePseries(def) < 0)
+goto error;
}
-if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
-goto error;
-
if (virDomainDeviceInfoParseXML(xmlopt, memdevNode,
&def->info, flags) < 0)
goto error;
diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
index ae5a17d3c8..ecb1b83b4a 100644
--- a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
+++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
@@ -38,7 +38,7 @@
/tmp/nvdimm
-55
+524416
0
128
--
2.26.2
[PATCH v1 2/4] conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c
We'll use the auto-alignment function during parse time, in
domain_conf.c. Let's move the function to that file, renaming
it to virDomainNVDimmAlignSizePseries(). This will also make it
clearer that, although QEMU is the only driver that currently
supports it, pSeries NVDIMM restrictions aren't tied to QEMU.
Signed-off-by: Daniel Henrique Barboza
---
src/conf/domain_conf.c | 36 ++
src/conf/domain_conf.h | 3 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain.c | 42 ++--
4 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 72ac4f4191..63434b9d3e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16876,6 +16876,42 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
return NULL;
}
+int
+virDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem)
+{
+/* For NVDIMMs in ppc64 in we want to align down the guest
+ * visible space, instead of align up, to avoid writing
+ * beyond the end of file by adding a potential 256MiB
+ * to the user specified size.
+ *
+ * The label-size is mandatory for ppc64 as well, meaning that
+ * the guest visible space will be target_size-label_size.
+ *
+ * Finally, target_size must include label_size.
+ *
+ * The above can be summed up as follows:
+ *
+ * target_size = AlignDown(target_size - label_size) + label_size
+ */
+unsigned long long ppc64AlignSize = 256 * 1024;
+unsigned long long guestArea = mem->size - mem->labelsize;
+
+/* Align down guest_area. 256MiB is the minimum size. Error
+ * out if target_size is smaller than 256MiB + label_size,
+ * since aligning it up will cause QEMU errors. */
+if (mem->size < (ppc64AlignSize + mem->labelsize)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("minimum target size for the NVDIMM "
+ "must be 256MB plus the label size"));
+return -1;
+}
+
+guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
+mem->size = guestArea + mem->labelsize;
+
+return 0;
+}
+
static virDomainMemoryDefPtr
virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr memdevNode,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14a376350c..19ee9d6832 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3895,6 +3895,9 @@ bool
virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a,
const virDomainBlockIoTuneInfo *b);
+int
+virDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem);
+
bool
virHostdevIsSCSIDevice(const virDomainHostdevDef *hostdev)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5f1aea3694..6bf2ef9744 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -542,6 +542,7 @@ virDomainNetTypeToString;
virDomainNetUpdate;
virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
+virDomainNVDimmAlignSizePseries;
virDomainObjAssignDef;
virDomainObjBroadcast;
virDomainObjCheckActive;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4f796bef4a..f50a0a4710 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8067,44 +8067,6 @@ qemuDomainGetMemoryModuleSizeAlignment(const
virDomainDef *def,
}
-static int
-qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def,
- virDomainMemoryDefPtr mem)
-{
-/* For NVDIMMs in ppc64 in we want to align down the guest
- * visible space, instead of align up, to avoid writing
- * beyond the end of file by adding a potential 256MiB
- * to the user specified size.
- *
- * The label-size is mandatory for ppc64 as well, meaning that
- * the guest visible space will be target_size-label_size.
- *
- * Finally, target_size must include label_size.
- *
- * The above can be summed up as follows:
- *
- * target_size = AlignDown(target_size - label_size) + label_size
- */
-unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def);
-unsigned long long guestArea = mem->size - mem->labelsize;
-
-/* Align down guest_area. 256MiB is the minimum size. Error
- * out if target_size is smaller than 256MiB + label_size,
- * since aligning it up will cause QEMU errors. */
-if (mem->size < (ppc64AlignSize + mem->labelsize)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("minimum target size for the NVDIMM "
- "must be 256MB plus the label size"));
-return -1;
-}
-
-guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
-mem->size = guestArea + mem->labelsize;
-
-return 0;
-}
-
-
int
qemuDomainAlignMemorySizes(virDomainDefPtr def)
{
@@ -8153,7 +8115,7 @@ qemuDomainAl
[PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes
In [1], changes were made to remove the existing auto-alignment
for pSeries NVDIMM devices. That design promotes strange situations
where the NVDIMM size reported in the domain XML is different
from what QEMU is actually using. We removed the auto-alignment
and relied on standard size validation.
However, this goes against Libvirt design philosophy of not
tampering with existing guest behavior, as pointed out by Daniel
in [2]. Since we can't know for sure whether there are guests that
are relying on the auto-alignment feature to work, the changes
made in [1] are a direct violation of this rule.
This patch reverts [1] entirely, re-enabling auto-alignment for
pSeries NVDIMM as it was before. Changes will be made to ease
the limitations of this design without hurting existing
guests.
This reverts the following commits:
- commit 0ee56369c8b4f2f898b6aa1ff1f51ab033be1c02,
"qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type".
- commit 07de813924caf37e535855541c0c1183d9d382e2,
"qemu_domain.c: do not auto-align ppc64 NVDIMMs"
- commit 0ccceaa57c50e5ee528f7073fa8723afd62b88b7,
"qemu_validate.c: add pSeries NVDIMM size alignment validation"
- commit 4fa2202d884414ad34d9952e72fb39b1d93c7e14,
"qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public"
- commit 2d93cbdea9d1b8dbf36bc0ffee6cb73d83d208c7,
"Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic""
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html
[2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html
---
docs/formatdomain.rst | 6 +-
src/qemu/qemu_domain.c| 57 +--
src/qemu/qemu_domain.h| 6 +-
src/qemu/qemu_hotplug.c | 6 +-
src/qemu/qemu_validate.c | 42 ++
...ory-hotplug-nvdimm-ppc64.ppc64-latest.args | 2 +-
.../memory-hotplug-nvdimm-ppc64.xml | 2 +-
.../memory-hotplug-nvdimm-ppc64.xml | 2 +-
8 files changed, 70 insertions(+), 53 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 18e237c157..d5930a4ac1 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -7216,8 +7216,10 @@ Example: usage of the memory devices
following restrictions apply:
#. the minimum label size is 128KiB,
- #. the remaining size (total-size - label-size) will be aligned
- to 4KiB as default.
+ #. the remaining size (total-size - label-size), also called guest area,
+ will be aligned to 4KiB as default. For pSeries guests, the guest area
+ will be aligned down to 256MiB, and the minimum size of the guest area
+ must be at least 256MiB.
``readonly``
The ``readonly`` element is used to mark the vNVDIMM as read-only. Only
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 03917cf000..4f796bef4a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8037,7 +8037,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
}
-unsigned long long
+static unsigned long long
qemuDomainGetMemorySizeAlignment(const virDomainDef *def)
{
/* PPC requires the memory sizes to be rounded to 256MiB increments, so
@@ -8067,6 +8067,44 @@ qemuDomainGetMemoryModuleSizeAlignment(const
virDomainDef *def,
}
+static int
+qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+/* For NVDIMMs in ppc64 in we want to align down the guest
+ * visible space, instead of align up, to avoid writing
+ * beyond the end of file by adding a potential 256MiB
+ * to the user specified size.
+ *
+ * The label-size is mandatory for ppc64 as well, meaning that
+ * the guest visible space will be target_size-label_size.
+ *
+ * Finally, target_size must include label_size.
+ *
+ * The above can be summed up as follows:
+ *
+ * target_size = AlignDown(target_size - label_size) + label_size
+ */
+unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def);
+unsigned long long guestArea = mem->size - mem->labelsize;
+
+/* Align down guest_area. 256MiB is the minimum size. Error
+ * out if target_size is smaller than 256MiB + label_size,
+ * since aligning it up will cause QEMU errors. */
+if (mem->size < (ppc64AlignSize + mem->labelsize)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("minimum target size for the NVDIMM "
+ "must be 256MB plus the label size"));
+return -1;
+}
+
+guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
+mem->size = guestArea + mem->labelsize;
+
+return 0;
+}
+
+
int
qemuDomainAlignMemorySizes(virDomainDefPtr def)
{
@@ -8113,8 +8151,11 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
/* Align memory module sizes */
for (i = 0; i < def->
Re: [PATCH 0/2] qemu: migration corner case fix and cleanup
There is issue with second patch. Probably this unset/set was too mysterious
and I miss one point - we do need to call unset on error path as in case
of non p2p migration confirm step will not be called.
So this need to be squashed in:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 01c702e..6213025 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4958,6 +4958,8 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
jobPriv->migParams, priv->job.apiFlags);
qemuMigrationJobFinish(driver, vm);
+virCloseCallbacksUnset(driver->closeCallbacks, vm,
+ qemuMigrationSrcCleanup);
} else {
qemuMigrationJobContinue(vm);
}
On 14.09.2020 18:41, Michal Privoznik wrote:
> On 8/18/20 12:26 PM, Nikolay Shirokovskiy wrote:
>> Nikolay Shirokovskiy (2):
>> qemu: fix qemuMigrationSrcCleanup to use qemuMigrationJobFinish
>> qemu: don't needlessly unset close callback during perform phase
>>
>> src/qemu/qemu_migration.c | 14 +++---
>> 1 file changed, 3 insertions(+), 11 deletions(-)
>>
>
> Patches look good to me, but since it was Jirka who added the call you're
> removing in 2/2 I'll let him share opinion too.
>
> Reviewed-by: Michal Privoznik
>
> Michal
>
[PATCH v2 0/5] Support CSS and DASD devices in node_device driver
This series enables support for channel subsystem (CSS) devices, Direct Access Storage Devices (DASDs) in the node_device driver and adds support to create vfio-ccw mdev devices in the node_device driver. Changed in V2: * now filtering only supported drivers in udevProcessCSS * renamed udevProcessDasd into udevProcessDASD * addend explaining comment in udevKludgeStorageType * generalized error message in nodeDeviceGetMdevctlStartCommand Boris Fiuczynski (5): node_device: refactor udevProcessCCW node_device: detect CSS devices virsh: nodedev: ability to filter CSS capabilities node_device: detect DASD devices node_device: mdev vfio-ccw support docs/formatnode.html.in | 12 +++ docs/manpages/virsh.rst | 2 +- docs/schemas/nodedev.rng | 16 include/libvirt/libvirt-nodedev.h | 1 + src/conf/domain_addr.c| 2 +- src/conf/domain_addr.h| 3 + src/conf/node_device_conf.c | 5 ++ src/conf/node_device_conf.h | 4 +- src/conf/virnodedeviceobj.c | 4 +- src/libvirt-nodedev.c | 1 + src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 26 -- src/node_device/node_device_udev.c| 80 --- .../ccw_0_0_1-invalid.xml | 4 +- tests/nodedevschemadata/ccw_0_0_.xml | 4 +- tests/nodedevschemadata/css_0_0_.xml | 10 +++ tests/nodedevxml2xmltest.c| 1 + tools/virsh-nodedev.c | 3 + 18 files changed, 153 insertions(+), 26 deletions(-) create mode 100644 tests/nodedevschemadata/css_0_0_.xml -- 2.25.1
[PATCH v2 4/5] node_device: detect DASD devices
Make Direct Access Storage Devices (DASDs) available in the node_device driver.
Reviewed-by: Bjoern Walk
Reviewed-by: Erik Skultety
Signed-off-by: Boris Fiuczynski
---
src/node_device/node_device_udev.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 38906f5f96..023377fc01 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,
}
+static int
+udevProcessDASD(struct udev_device *device,
+virNodeDeviceDefPtr def)
+{
+virNodeDevCapStoragePtr storage = &def->caps->data.storage;
+
+if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial) < 0)
+return -1;
+
+return udevProcessDisk(device, def);
+}
+
+
/* This function exists to deal with the case in which a driver does
* not provide a device type in the usual place, but udev told us it's
* a storage device, and we can make a good guess at what kind of
@@ -891,6 +904,18 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
def->sysfs_path);
return 0;
}
+/* For Direct Access Storage Devices (DASDs) there are
+ * currently no identifies in udev besides ID_PATH. Since
+ * ID_TYPE=disk does not exist on DASDs they fall through
+ * the udevProcessStorage detection logic. */
+if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+def->caps->data.storage.drive_type = g_strdup("dasd");
+VIR_DEBUG("Found storage type '%s' for device "
+ "with sysfs path '%s'",
+ def->caps->data.storage.drive_type,
+ def->sysfs_path);
+return 0;
+}
VIR_DEBUG("Could not determine storage type "
"for device with sysfs path '%s'", def->sysfs_path);
return -1;
@@ -978,6 +1003,8 @@ udevProcessStorage(struct udev_device *device,
ret = udevProcessFloppy(device, def);
} else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
ret = udevProcessSD(device, def);
+} else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
+ret = udevProcessDASD(device, def);
} else {
VIR_DEBUG("Unsupported storage type '%s'",
def->caps->data.storage.drive_type);
--
2.25.1
[PATCH v2 2/5] node_device: detect CSS devices
Make channel subsystem (CSS) devices available in the node_device driver.
The CCS devices reside in the computer system and provide CCW devices, e.g.:
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
|
+- scsi_target0_0_0
|
+- scsi_0_0_0_0
Reviewed-by: Bjoern Walk
Signed-off-by: Boris Fiuczynski
---
docs/schemas/nodedev.rng | 16 ++
src/conf/node_device_conf.c | 5 +
src/conf/node_device_conf.h | 1 +
src/conf/virnodedeviceobj.c | 1 +
src/node_device/node_device_udev.c| 22 +++
.../ccw_0_0_1-invalid.xml | 4 ++--
tests/nodedevschemadata/ccw_0_0_.xml | 4 ++--
tests/nodedevschemadata/css_0_0_.xml | 10 +
tests/nodedevxml2xmltest.c| 1 +
tools/virsh-nodedev.c | 1 +
10 files changed, 61 insertions(+), 4 deletions(-)
create mode 100644 tests/nodedevschemadata/css_0_0_.xml
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 4b2b350fd8..f7f517b548 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -85,6 +85,7 @@
+
@@ -659,6 +660,21 @@
+
+
+ css
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index f6a91165c9..a9a03ad6c2 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -65,6 +65,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
"mdev_types",
"mdev",
"ccw",
+ "css",
);
VIR_ENUM_IMPL(virNodeDevNetCap,
@@ -602,6 +603,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
virNodeDeviceCapMdevDefFormat(&buf, data);
break;
case VIR_NODE_DEV_CAP_CCW_DEV:
+case VIR_NODE_DEV_CAP_CSS_DEV:
virBufferAsprintf(&buf, "0x%x\n",
data->ccw_dev.cssid);
virBufferAsprintf(&buf, "0x%x\n",
@@ -1904,6 +1906,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
ret = virNodeDevCapMdevParseXML(ctxt, def, node, &caps->data.mdev);
break;
case VIR_NODE_DEV_CAP_CCW_DEV:
+case VIR_NODE_DEV_CAP_CSS_DEV:
ret = virNodeDevCapCCWParseXML(ctxt, def, node, &caps->data.ccw_dev);
break;
case VIR_NODE_DEV_CAP_MDEV_TYPES:
@@ -2228,6 +2231,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
case VIR_NODE_DEV_CAP_FC_HOST:
case VIR_NODE_DEV_CAP_VPORTS:
case VIR_NODE_DEV_CAP_CCW_DEV:
+case VIR_NODE_DEV_CAP_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
/* This case is here to shutup the compiler */
break;
@@ -2281,6 +2285,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
case VIR_NODE_DEV_CAP_MDEV_TYPES:
case VIR_NODE_DEV_CAP_MDEV:
case VIR_NODE_DEV_CAP_CCW_DEV:
+case VIR_NODE_DEV_CAP_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
break;
}
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 9b8c7aadea..47669d4294 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -64,6 +64,7 @@ typedef enum {
VIR_NODE_DEV_CAP_MDEV_TYPES,/* Device capable of mediated devices
*/
VIR_NODE_DEV_CAP_MDEV, /* Mediated device */
VIR_NODE_DEV_CAP_CCW_DEV, /* s390 CCW device */
+VIR_NODE_DEV_CAP_CSS_DEV, /* s390 channel subsystem device */
VIR_NODE_DEV_CAP_LAST
} virNodeDevCapType;
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index bfd524121c..e234432b6f 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -710,6 +710,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
case VIR_NODE_DEV_CAP_MDEV_TYPES:
case VIR_NODE_DEV_CAP_MDEV:
case VIR_NODE_DEV_CAP_CCW_DEV:
+case VIR_NODE_DEV_CAP_CSS_DEV:
case VIR_NODE_DEV_CAP_LAST:
break;
}
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index d478a673fd..38906f5f96 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1097,6 +1097,24 @@ udevProcessCCW(struct udev_device *device,
}
+static int
+udevProcessCSS(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+/* only process IO subchannel and vfio-ccw devices to keep the list sane */
+if (STRNEQ(def->driver, "io_subchannel") &&
+STRNEQ(def->driver, "vfio_ccw"))
+return -1;
+
+if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
+return -1;
+
+if (udevGenerateDeviceName(device, def, NULL) != 0)
+return -1;
+
+return 0;
+}
+
static
[PATCH v2 3/5] virsh: nodedev: ability to filter CSS capabilities
Allow to filter for CSS devices.
Reviewed-by: Bjoern Walk
Reviewed-by: Erik Skultety
Signed-off-by: Boris Fiuczynski
---
docs/formatnode.html.in | 12
docs/manpages/virsh.rst | 2 +-
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/node_device_conf.h | 3 ++-
src/conf/virnodedeviceobj.c | 3 ++-
src/libvirt-nodedev.c | 1 +
tools/virsh-nodedev.c | 2 ++
7 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 8a51c4da80..4c7de50a0b 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -420,6 +420,18 @@
The device number.
+ css
+ Describes a Channel SubSystem (CSS) device commonly found on
+ the S390 architecture. Sub-elements include:
+
+ cssid
+ The channel subsystem identifier.
+ ssid
+ The subchannel-set identifier.
+ devno
+ The device number.
+
+
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index dde572ac79..9fd5efed0f 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4964,7 +4964,7 @@ List all of the devices available on the node that are
known by libvirt.
separated by comma, e.g. --cap pci,scsi. Valid capability types include
'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target',
'scsi', 'storage', 'fc_host', 'vports', 'scsi_generic', 'drm', 'mdev',
-'mdev_types', 'ccw'.
+'mdev_types', 'ccw', 'css'.
If *--tree* is used, the output is formatted in a tree representing parents of
each
node. *cap* and *--tree* are mutually exclusive.
diff --git a/include/libvirt/libvirt-nodedev.h
b/include/libvirt/libvirt-nodedev.h
index a2ad61ac6d..dd2ffd5782 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -81,6 +81,7 @@ typedef enum {
VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES= 1 << 13, /* Capable of
mediated devices */
VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV = 1 << 14, /* Mediated
device */
VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV = 1 << 15, /* CCW device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV = 1 << 16, /* CSS device */
} virConnectListAllNodeDeviceFlags;
int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 47669d4294..5484bc340f 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -368,7 +368,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM | \
VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES| \
VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV | \
- VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV)
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV | \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV)
int
virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index e234432b6f..8aefd15e94 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -861,7 +861,8 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
MATCH(DRM) ||
MATCH(MDEV_TYPES)||
MATCH(MDEV) ||
- MATCH(CCW_DEV)))
+ MATCH(CCW_DEV) ||
+ MATCH(CSS_DEV)))
return false;
}
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index ca6c2bb057..28765b9c30 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -101,6 +101,7 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap,
unsigned int flags)
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV
* VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV
+ * VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV
*
* Returns the number of node devices found or -1 and sets @devices to NULL in
* case of error. On success, the array stored into @devices is guaranteed to
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index d497fa9797..2edd403a64 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -462,6 +462,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd
G_GNUC_UNUSED)
flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV;
break;
case VIR_NODE_DEV_CAP_CSS_DEV:
+flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV;
+break;
case VIR_NODE_DEV_CAP_LAST:
break;
}
--
2.25.1
[PATCH v2 1/5] node_device: refactor udevProcessCCW
Refactor out CCW address parsing for later reuse.
Reviewed-by: Erik Skultety
Reviewed-by: Bjoern Walk
Signed-off-by: Boris Fiuczynski
---
src/node_device/node_device_udev.c | 31 --
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index ff558efb83..d478a673fd 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1058,27 +1058,38 @@ udevProcessMediatedDevice(struct udev_device *dev,
static int
-udevProcessCCW(struct udev_device *device,
- virNodeDeviceDefPtr def)
+udevGetCCWAddress(const char *sysfs_path,
+ virNodeDevCapDataPtr data)
{
-int online;
char *p;
-virNodeDevCapDataPtr data = &def->caps->data;
-
-/* process only online devices to keep the list sane */
-if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online != 1)
-return -1;
-if ((p = strrchr(def->sysfs_path, '/')) == NULL ||
+if ((p = strrchr(sysfs_path, '/')) == NULL ||
virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.cssid) < 0 || p == NULL
||
virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.ssid) < 0 || p == NULL ||
virStrToLong_ui(p + 1, &p, 16, &data->ccw_dev.devno) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to parse the CCW address from sysfs path:
'%s'"),
- def->sysfs_path);
+ sysfs_path);
return -1;
}
+return 0;
+}
+
+
+static int
+udevProcessCCW(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+int online;
+
+/* process only online devices to keep the list sane */
+if (udevGetIntSysfsAttr(device, "online", &online, 0) < 0 || online != 1)
+return -1;
+
+if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
+return -1;
+
if (udevGenerateDeviceName(device, def, NULL) != 0)
return -1;
--
2.25.1
[PATCH v2 5/5] node_device: mdev vfio-ccw support
Allow vfio-ccw mdev devices to be created besides vfio-pci mdev devices
as well.
Reviewed-by: Bjoern Walk
Signed-off-by: Boris Fiuczynski
---
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 3 +++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 26 ++
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 1068cbf1d2..1bfa164a47 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1365,7 +1365,7 @@ virDomainPCIAddressSetAllMulti(virDomainDefPtr def)
}
-static char*
+char*
virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
{
return g_strdup_printf("%x.%x.%04x", addr->cssid, addr->ssid, addr->devno);
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index c1363c1490..a0460b4030 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -208,6 +208,9 @@ int virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs);
+char* virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
+ATTRIBUTE_NONNULL(1);
+
virDomainCCWAddressSetPtr
virDomainCCWAddressSetCreateFromDomain(virDomainDefPtr def)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5f1aea3694..5842b8d23d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -143,6 +143,7 @@ virPCIDeviceAddressParseXML;
# conf/domain_addr.h
virDomainCCWAddressAssign;
+virDomainCCWAddressAsString;
virDomainCCWAddressSetCreateFromDomain;
virDomainCCWAddressSetFree;
virDomainPCIAddressBusIsFullyReserved;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index e89c8b0ee5..375c1fff32 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -28,6 +28,7 @@
#include "virerror.h"
#include "datatypes.h"
+#include "domain_addr.h"
#include "viralloc.h"
#include "virfile.h"
#include "virjson.h"
@@ -628,7 +629,7 @@ nodeDeviceFindAddressByName(const char *name)
{
virNodeDeviceDefPtr def = NULL;
virNodeDevCapsDefPtr caps = NULL;
-char *pci_addr = NULL;
+char *addr = NULL;
virNodeDeviceObjPtr dev = virNodeDeviceObjListFindByName(driver->devs,
name);
if (!dev) {
@@ -640,21 +641,30 @@ nodeDeviceFindAddressByName(const char *name)
def = virNodeDeviceObjGetDef(dev);
for (caps = def->caps; caps != NULL; caps = caps->next) {
if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
-virPCIDeviceAddress addr = {
+virPCIDeviceAddress pci_addr = {
.domain = caps->data.pci_dev.domain,
.bus = caps->data.pci_dev.bus,
.slot = caps->data.pci_dev.slot,
.function = caps->data.pci_dev.function
};
-pci_addr = virPCIDeviceAddressAsString(&addr);
+addr = virPCIDeviceAddressAsString(&pci_addr);
+break;
+} else if (caps->data.type == VIR_NODE_DEV_CAP_CSS_DEV) {
+virDomainDeviceCCWAddress ccw_addr = {
+.cssid = caps->data.ccw_dev.cssid,
+.ssid = caps->data.ccw_dev.ssid,
+.devno = caps->data.ccw_dev.devno
+};
+
+addr = virDomainCCWAddressAsString(&ccw_addr);
break;
}
}
virNodeDeviceObjEndAPI(&dev);
-return pci_addr;
+return addr;
}
@@ -664,11 +674,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
{
virCommandPtr cmd;
g_autofree char *json = NULL;
-g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
-if (!parent_pci) {
+if (!parent_addr) {
virReportError(VIR_ERR_NO_NODE_DEVICE,
- _("unable to find PCI address for parent device '%s'"),
def->parent);
+ _("unable to find parent device '%s'"), def->parent);
return NULL;
}
@@ -679,7 +689,7 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
}
cmd = virCommandNewArgList(MDEVCTL, "start",
- "-p", parent_pci,
+ "-p", parent_addr,
"--jsonfile", "/dev/stdin",
NULL);
--
2.25.1
Re: [libvirt PATCH 0/3] virsh-completer: Use Glib memory functions (glib chronicles)
On 9/14/20 6:08 PM, Ján Tomko wrote: Ján Tomko (3): virsh-completer: use g_clear_pointer in virshCommaStringListComplete virsh-completer: use g_free instead of VIR_FREE virsh-completer: use g_new0 instead of VIR_ALLOC_N tools/virsh-completer-checkpoint.c | 11 tools/virsh-completer-domain.c | 44 ++ tools/virsh-completer-host.c | 6 ++-- tools/virsh-completer-interface.c | 6 ++-- tools/virsh-completer-network.c| 25 +++-- tools/virsh-completer-nodedev.c| 12 +++- tools/virsh-completer-nwfilter.c | 12 +++- tools/virsh-completer-pool.c | 12 +++- tools/virsh-completer-secret.c | 8 ++ tools/virsh-completer-snapshot.c | 5 ++-- tools/virsh-completer-volume.c | 5 ++-- tools/virsh-completer.c| 5 ++-- 12 files changed, 55 insertions(+), 96 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH] util: virNetDevTapCreate: initialize fd to -1
derp! :-O On 9/14/20 7:03 AM, Ján Tomko wrote: Signed-off-by: Ján Tomko Fixes: 95089f481e003d971fe0a082018216c58c1b80e5 --- Pushed as trivial. src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index ab5959c646..cbce5c71b7 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -322,7 +322,7 @@ int virNetDevTapCreate(char **ifname, size_t i = 0; struct ifreq ifr; int ret = -1; -int fd = 0; +int fd = -1; virMutexLock(&virNetDevTapCreateMutex);
[libvirt PATCH v2 2/2] esx: implement domainInterfaceAddresses
Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.
Signed-off-by: Pino Toscano
---
docs/drvesx.html.in | 4 +
src/esx/esx_driver.c | 170 +++
2 files changed, 174 insertions(+)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 38de640c2a..3acb7e5c51 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -799,6 +799,10 @@ Enter administrator password for example-vcenter.com:
virDomainGetHostname
+
+virDomainInterfaceAddresses (only for the
+VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+
virDomainReboot
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index bddc588977..5a742ed3df 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,175 @@ esxDomainGetHostname(virDomainPtr domain,
}
+static int
+esxParseIPAddress(const char *ipAddress, int prefixLength,
+ virDomainIPAddress *addr)
+{
+virSocketAddr tmp_addr;
+virIPAddrType addr_type;
+
+if (virSocketAddrParseAny(&tmp_addr, ipAddress, AF_UNSPEC, false) <= 0)
+return 0;
+
+switch (VIR_SOCKET_ADDR_FAMILY(&tmp_addr)) {
+case AF_INET:
+addr_type = VIR_IP_ADDR_TYPE_IPV4;
+break;
+case AF_INET6:
+addr_type = VIR_IP_ADDR_TYPE_IPV6;
+break;
+default:
+return 0;
+}
+
+addr->type = addr_type;
+addr->addr = g_strdup(ipAddress);
+addr->prefix = prefixLength;
+
+return 1;
+}
+
+
+static int
+esxDomainInterfaceAddresses(virDomainPtr domain,
+virDomainInterfacePtr **ifaces,
+unsigned int source,
+unsigned int flags)
+{
+int result = -1;
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+esxVI_DynamicProperty *dynamicProperty;
+esxVI_GuestNicInfo *guestNicInfoList = NULL;
+esxVI_GuestNicInfo *guestNicInfo = NULL;
+virDomainInterfacePtr *ifaces_ret = NULL;
+size_t ifaces_count = 0;
+size_t i;
+int ret;
+
+virCheckFlags(0, -1);
+if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Unknown IP address data source %d"),
+ source);
+return -1;
+}
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+if (esxVI_String_AppendValueListToList(&propertyNameList,
+ "runtime.powerState\0"
+ "guest.net") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, &virtualMachine,
+ esxVI_Occurrence_RequiredItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not powered on"));
+goto cleanup;
+}
+
+for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "guest.net")) {
+if (esxVI_GuestNicInfo_CastListFromAnyType
+ (dynamicProperty->val, &guestNicInfoList) < 0) {
+goto cleanup;
+}
+}
+}
+
+if (!guestNicInfoList) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Missing property '%s' in answer"),
+ "guest.net");
+goto cleanup;
+}
+
+for (guestNicInfo = guestNicInfoList; guestNicInfo;
+ guestNicInfo = guestNicInfo->_next) {
+virDomainInterfacePtr iface = NULL;
+size_t addrs_count = 0;
+
+if (guestNicInfo->connected != esxVI_Boolean_True ||
+!guestNicInfo->network) {
+continue;
+}
+
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface->naddrs = 0;
+iface->name = g_strdup(guestNicInfo->network);
+iface->hwaddr = g_strdup(guestNicInfo->macAddress);
+
+if (guestNicInfo->ipConfig) {
+esxVI_NetIpConfigInfoIpAddress *ipAddress;
+for (ipAddress = guestNicInfo->ipConfig->ipAddress; ipAddress;
+ ipAddress = ipAddress->_next) {
+virDomainIPAddress ip_addr;
+
+ret = esxParseIPAddres
[libvirt PATCH v2 1/2] esx: generator: add GuestNicInfo object
Add the definition of the GuestNicInfo object, with all the required
objects for it.
Signed-off-by: Pino Toscano
---
scripts/esx_vi_generator.py| 1 +
src/esx/esx_vi_generator.input | 54 ++
2 files changed, 55 insertions(+)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index e0782e35f3..7929e1e682 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1292,6 +1292,7 @@ additional_object_features = {
"DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST |
Object.FEATURE__ANY_TYPE),
"DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST,
+"GuestNicInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
"HostConfigManager": Object.FEATURE__ANY_TYPE,
"HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
"HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST |
diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input
index 22c114e0aa..bd6ac72a18 100644
--- a/src/esx/esx_vi_generator.input
+++ b/src/esx/esx_vi_generator.input
@@ -277,6 +277,18 @@ object FolderFileQuery extends FileQuery
end
+object GuestNicInfo
+Boolean connected r
+Int deviceConfigId r
+NetDnsConfigInfo dnsConfig o
+String ipAddress ol
+NetIpConfigInfo ipConfig o
+String macAddress o
+NetBIOSConfigInfonetBIOSConfig o
+String networko
+end
+
+
object HostAutoStartManagerConfig
AutoStartDefaultsdefaults o
AutoStartPowerInfo powerInfo ol
@@ -770,6 +782,48 @@ object NasDatastoreInfo extends DatastoreInfo
end
+object NetBIOSConfigInfo
+String mode r
+end
+
+
+object NetDhcpConfigInfo
+NetDhcpConfigInfoDhcpOptions ipv4 o
+NetDhcpConfigInfoDhcpOptions ipv6 o
+end
+
+
+object NetDhcpConfigInfoDhcpOptions
+KeyAnyValue config ol
+Boolean enable r
+end
+
+
+object NetDnsConfigInfo
+Boolean dhcp r
+String domainName r
+String hostName r
+String ipAddress ol
+String searchDomain ol
+end
+
+
+object NetIpConfigInfo
+Boolean autoConfigurationEnabled o
+NetDhcpConfigInfodhcp o
+NetIpConfigInfoIpAddress ipAddress ol
+end
+
+
+object NetIpConfigInfoIpAddress
+String ipAddress r
+DateTime lifetime o
+String origin o
+Int prefixLength r
+String state o
+end
+
+
object ObjectContent
ManagedObjectReference objr
DynamicProperty propSetol
--
2.26.2
Re: [libvirt PATCH 2/2] esx: implement domainInterfaceAddresses
On Monday, 14 September 2020 17:33:02 CEST Michal Privoznik wrote:
> > +for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
> > + dynamicProperty = dynamicProperty->_next) {
> > +if (STREQ(dynamicProperty->name, "guest.net")) {
> > +if (esxVI_GuestNicInfo_CastListFromAnyType
> > + (dynamicProperty->val, &guestNicInfoList) < 0) {
> > +goto cleanup;
> > +}
> > +}
> > +}
> > +
> > +if (!guestNicInfoList)
> > +goto cleanup;
>
> This looks suspicious. If I understand the code correctly then this
> means we haven't found any network config. What we usually do is we
> return an empty array (*ifaces = NULL) and return 0. But if this is a
> more serious error then we need a virReportError() here.
Good notice, it is actually an issue (we requested a property of a VM,
the SOAP call for it succeeded but there was no property in the
answer). I will amend and send v2.
Thanks,
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH 2/3] qemu: backup: Remove note that TLS should be implemented
Commit 423576679a5 implementing TLS forgot to remove the comment.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index a402730d38..2f1a612803 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -805,7 +805,6 @@ qemuBackupBegin(virDomainObjPtr vm,
if (qemuDomainObjEnterMonitorAsync(priv->driver, vm,
QEMU_ASYNC_JOB_BACKUP) < 0)
goto endjob;
-/* TODO: TLS is a must-have for the modern age */
if (pull) {
if (tlsSecretProps)
rc = qemuMonitorAddObject(priv->mon, &tlsSecretProps,
&tlsSecretAlias);
--
2.26.2
[libvirt PATCH v2] esx: implement connectListAllNetworks
Implement the .connectListAllNetworks networks API in the esx network
driver.
Signed-off-by: Pino Toscano
---
src/esx/esx_network_driver.c | 69
1 file changed, 69 insertions(+)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 88843aae2f..a4e738ba72 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,74 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+ virNetworkPtr **nets,
+ unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+ &hostVirtualSwitchList) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+if (nets) {
+virNetworkPtr net = virtualswitchToNetwork(conn,
hostVirtualSwitch);
+if (!net)
+goto cleanup;
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+} else {
+++count;
+}
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (nets && *nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free(&hostVirtualSwitchList);
+
+return ret;
+}
+#undef MATCH
+
+
+
virNetworkDriver esxNetworkDriver = {
.connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
.connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +945,5 @@ virNetworkDriver esxNetworkDriver = {
.networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
.networkIsActive = esxNetworkIsActive, /* 0.10.0 */
.networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.8.0 */
};
--
2.26.2
[PATCH 3/3] qemu: backup: Write TLS cert and secret object aliases into status XML
We've put the aliases into the backup job definition after the status
XML was already writted so they didn't appear in the on-disk state.
Move the code putting them into the private definition earlier, so that
the status XML update done by saving blockjobs already writes them out.
Also add a note notifying that the block job status update writes the
status XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870488
Fixes: 423576679a5
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 2f1a612803..4e61a5e52b 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -825,6 +825,9 @@ qemuBackupBegin(virDomainObjPtr vm,
goto endjob;
job_started = true;
+priv->backup->tlsAlias = g_steal_pointer(&tlsAlias);
+priv->backup->tlsSecretAlias = g_steal_pointer(&tlsSecretAlias);
+/* qemuBackupDiskStarted saves the status XML */
qemuBackupDiskStarted(vm, dd, ndd);
if (chk) {
@@ -848,9 +851,6 @@ qemuBackupBegin(virDomainObjPtr vm,
}
}
-priv->backup->tlsAlias = g_steal_pointer(&tlsAlias);
-priv->backup->tlsSecretAlias = g_steal_pointer(&tlsSecretAlias);
-
ret = 0;
endjob:
--
2.26.2
[PATCH 0/3] qemu: backup: Properly write TLS cert and secret object alias into status XML
Please see 3/3 for explanation. Peter Krempa (3): qemustatusxml2xml: backup-pull: Test private data formatting/parsing qemu: backup: Remove note that TLS should be implemented qemu: backup: Write TLS cert and secret object aliases into status XML src/qemu/qemu_backup.c | 7 +++ tests/qemustatusxml2xmldata/backup-pull-in.xml | 8 +++- 2 files changed, 10 insertions(+), 5 deletions(-) -- 2.26.2
[PATCH 1/3] qemustatusxml2xml: backup-pull: Test private data formatting/parsing
Modify the test case to enable TLS and add private data containing aliases of objects corresponding to a TLS setup. Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmldata/backup-pull-in.xml | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/qemustatusxml2xmldata/backup-pull-in.xml b/tests/qemustatusxml2xmldata/backup-pull-in.xml index 1db978a3ac..faaed67e38 100644 --- a/tests/qemustatusxml2xmldata/backup-pull-in.xml +++ b/tests/qemustatusxml2xmldata/backup-pull-in.xml @@ -254,12 +254,18 @@ 12345 - + + + + + + + -- 2.26.2
Re: [libvirt PATCH] esx: implement connectListAllNetworks
On Monday, 14 September 2020 17:16:50 CEST Michal Privoznik wrote:
> On 9/14/20 11:10 AM, Pino Toscano wrote:
> > Implement the .connectListAllNetworks networks API in the esx network
> > driver.
> >
> > Signed-off-by: Pino Toscano
> > ---
> > src/esx/esx_network_driver.c | 64
> > 1 file changed, 64 insertions(+)
> >
> > diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
> > index 88843aae2f..5d9c1e3c2c 100644
> > --- a/src/esx/esx_network_driver.c
> > +++ b/src/esx/esx_network_driver.c
> > @@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network
> > G_GNUC_UNUSED)
> >
> >
> >
> > +#define MATCH(FLAG) (flags & (FLAG))
> > +static int
> > +esxConnectListAllNetworks(virConnectPtr conn,
> > + virNetworkPtr **nets,
> > + unsigned int flags)
> > +{
> > +int ret = -1;
> > +esxPrivate *priv = conn->privateData;
> > +esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
> > +esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
> > +size_t count = 0;
> > +size_t i;
> > +
> > +virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
> > +
> > +/*
> > + * ESX networks are always active, persistent, and
> > + * autostarted, so return zero elements in case we are asked
> > + * for networks different than that.
> > + */
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
> > +return 0;
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
> > +return 0;
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
> > +return 0;
> > +
> > +if (esxVI_EnsureSession(priv->primary) < 0 ||
> > +esxVI_LookupHostVirtualSwitchList(priv->primary,
> > + &hostVirtualSwitchList) < 0) {
> > +return -1;
> > +}
> > +
> > +for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
> > + hostVirtualSwitch = hostVirtualSwitch->_next) {
> > +virNetworkPtr net = virtualswitchToNetwork(conn,
> > hostVirtualSwitch);
> > +
> > +if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
>
> The virConnectListAllNetworks() documents that @nets can be NULL if the
> caller is interested only in the count of networks.
Ah, I missed that bit, thanks. I will send v2 with a tweaked version.
Thanks,
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH] qemu: fix concurrency crash bug in force snapshot revert
This patch is just revert of [1]. Actually we should NOT pass QEMU_ASYNC_JOB_NONE as that patch suggests while we are in async job in order to acquire nested jobs correctly. The patch tries to fix issues introduced by another patch [2] where jobs are mistakenly cleared out in qemuProcessStop. Later patch [3] fixed the issue introduced by patch [2]. Now we need to revert [1] as well as we now still have same concurrency crash issues as [3] described but for the force revert. [1] 0c4408c83: qemu: Don't use asyncJob after stop during snapshot revert [2] 888aa4b6b: qemuDomainObjPrivateDataClear: Don't leak @migParams [3] d75f865fb: qemu: fix concurrency crash bug in snapshot revert Signed-off-by: Nikolay Shirokovskiy --- src/qemu/qemu_snapshot.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 1e8ea80..5f49fd1 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1719,7 +1719,6 @@ qemuSnapshotRevert(virDomainObjPtr vm, qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID; -qemuDomainAsyncJob jobType = QEMU_ASYNC_JOB_START; bool defined = false; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | @@ -1899,9 +1898,6 @@ qemuSnapshotRevert(virDomainObjPtr vm, VIR_DOMAIN_EVENT_STOPPED, detail); virObjectEventStateQueue(driver->domainEventState, event); -/* Start after stop won't be an async start job, so - * reset to none */ -jobType = QEMU_ASYNC_JOB_NONE; goto load; } } @@ -1968,7 +1964,7 @@ qemuSnapshotRevert(virDomainObjPtr vm, rc = qemuProcessStart(snapshot->domain->conn, driver, vm, cookie ? cookie->cpu : NULL, - jobType, NULL, -1, NULL, snap, + QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); @@ -2003,7 +1999,7 @@ qemuSnapshotRevert(virDomainObjPtr vm, } rc = qemuProcessStartCPUs(driver, vm, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, - jobType); + QEMU_ASYNC_JOB_START); if (rc < 0) goto endjob; virObjectUnref(event); -- 1.8.3.1
[libvirt PATCH 3/3] virsh-completer: use g_new0 instead of VIR_ALLOC_N
Signed-off-by: Ján Tomko
---
tools/virsh-completer-checkpoint.c | 3 +--
tools/virsh-completer-domain.c | 40 ++
tools/virsh-completer-host.c | 6 ++---
tools/virsh-completer-interface.c | 4 +--
tools/virsh-completer-network.c| 13 +++---
tools/virsh-completer-nodedev.c| 10 +++-
tools/virsh-completer-nwfilter.c | 8 ++
tools/virsh-completer-pool.c | 10 +++-
tools/virsh-completer-secret.c | 6 ++---
tools/virsh-completer-snapshot.c | 3 +--
tools/virsh-completer-volume.c | 3 +--
tools/virsh-completer.c| 3 +--
12 files changed, 34 insertions(+), 75 deletions(-)
diff --git a/tools/virsh-completer-checkpoint.c
b/tools/virsh-completer-checkpoint.c
index 0b5af2fa6b..29d644dad0 100644
--- a/tools/virsh-completer-checkpoint.c
+++ b/tools/virsh-completer-checkpoint.c
@@ -50,8 +50,7 @@ virshCheckpointNameCompleter(vshControl *ctl,
flags)) < 0)
goto error;
-if (VIR_ALLOC_N(ret, ncheckpoints + 1) < 0)
-goto error;
+ret = g_new0(char *, ncheckpoints + 1);
for (i = 0; i < ncheckpoints; i++) {
const char *name = virDomainCheckpointGetName(checkpoints[i]);
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 8159a0a58d..50a8a31fcd 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -61,8 +61,7 @@ virshDomainNameCompleter(vshControl *ctl,
if ((ndomains = virConnectListAllDomains(priv->conn, &domains, flags)) < 0)
return NULL;
-if (VIR_ALLOC_N(tmp, ndomains + 1) < 0)
-goto cleanup;
+tmp = g_new0(char *, ndomains + 1);
for (i = 0; i < ndomains; i++) {
const char *name = virDomainGetName(domains[i]);
@@ -72,7 +71,6 @@ virshDomainNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
- cleanup:
for (i = 0; i < ndomains; i++)
virshDomainFree(domains[i]);
g_free(domains);
@@ -110,8 +108,7 @@ virshDomainUUIDCompleter(vshControl *ctl,
if ((ndomains = virConnectListAllDomains(priv->conn, &domains, flags)) < 0)
return NULL;
-if (VIR_ALLOC_N(tmp, ndomains + 1) < 0)
-goto cleanup;
+tmp = g_new0(char *, ndomains + 1);
for (i = 0; i < ndomains; i++) {
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -161,8 +158,7 @@ virshDomainInterfaceCompleter(vshControl *ctl,
if (ninterfaces < 0)
return NULL;
-if (VIR_ALLOC_N(tmp, ninterfaces + 1) < 0)
-return NULL;
+tmp = g_new0(char *, ninterfaces + 1);
for (i = 0; i < ninterfaces; i++) {
ctxt->node = interfaces[i];
@@ -206,8 +202,7 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
if (ndisks < 0)
return NULL;
-if (VIR_ALLOC_N(tmp, ndisks + 1) < 0)
-return NULL;
+tmp = g_new0(char *, ndisks + 1);
for (i = 0; i < ndisks; i++) {
ctxt->node = disks[i];
@@ -229,8 +224,7 @@ virshDomainEventNameCompleter(vshControl *ctl G_GNUC_UNUSED,
virCheckFlags(0, NULL);
-if (VIR_ALLOC_N(tmp, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
-return NULL;
+tmp = g_new0(char *, VIR_DOMAIN_EVENT_ID_LAST + 1);
for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++)
tmp[i] = g_strdup(virshDomainEventCallbacks[i].name);
@@ -283,8 +277,7 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
ctxt->node = interfaces[0];
-if (VIR_ALLOC_N(tmp, 2) < 0)
-return NULL;
+tmp = g_new0(char *, 2);
if ((state = virXPathString("string(./link/@state)", ctxt)) &&
STREQ(state, "down")) {
@@ -326,8 +319,7 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
if (naliases < 0)
return NULL;
-if (VIR_ALLOC_N(tmp, naliases + 1) < 0)
-return NULL;
+tmp = g_new0(char *, naliases + 1);
for (i = 0; i < naliases; i++) {
if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
@@ -404,8 +396,7 @@ virshDomainPerfEnableCompleter(vshControl *ctl,
virCheckFlags(0, NULL);
-if (VIR_ALLOC_N(events, VIR_PERF_EVENT_LAST + 1) < 0)
-return NULL;
+events = g_new0(char *, VIR_PERF_EVENT_LAST + 1);
for (i = 0; i < VIR_PERF_EVENT_LAST; i++)
events[i] = g_strdup(virPerfEventTypeToString(i));
@@ -428,8 +419,7 @@ virshDomainPerfDisableCompleter(vshControl *ctl,
virCheckFlags(0, NULL);
-if (VIR_ALLOC_N(events, VIR_PERF_EVENT_LAST + 1) < 0)
-return NULL;
+events = g_new0(char *, VIR_PERF_EVENT_LAST + 1);
for (i = 0; i < VIR_PERF_EVENT_LAST; i++)
events[i] = g_strdup(virPerfEventTypeToString(i));
@@ -464,8 +454,7 @@ virshDomainIOThreadIdCompleter(vshControl *ctl,
niothreads = rc;
-if (VIR_ALLOC_N(tmp, niothreads + 1) < 0)
-goto cleanup;
+tmp = g_new0(char *, niothreads + 1);
for (i = 0; i < niothreads; i++)
tmp[i] = g_strdup_printf("%u", info[i]->iothread_id);
@@ -504,8 +493,7
[libvirt PATCH 2/3] virsh-completer: use g_free instead of VIR_FREE
All of them are in the cleanup section right before the variables they free go out of scope. Signed-off-by: Ján Tomko --- tools/virsh-completer-checkpoint.c | 8 tools/virsh-completer-domain.c | 4 ++-- tools/virsh-completer-interface.c | 2 +- tools/virsh-completer-network.c| 12 ++-- tools/virsh-completer-nodedev.c| 2 +- tools/virsh-completer-nwfilter.c | 4 ++-- tools/virsh-completer-pool.c | 2 +- tools/virsh-completer-secret.c | 2 +- tools/virsh-completer-snapshot.c | 2 +- tools/virsh-completer-volume.c | 2 +- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/tools/virsh-completer-checkpoint.c b/tools/virsh-completer-checkpoint.c index 21e8bb5d3b..0b5af2fa6b 100644 --- a/tools/virsh-completer-checkpoint.c +++ b/tools/virsh-completer-checkpoint.c @@ -60,7 +60,7 @@ virshCheckpointNameCompleter(vshControl *ctl, virshDomainCheckpointFree(checkpoints[i]); } -VIR_FREE(checkpoints); +g_free(checkpoints); virshDomainFree(dom); return ret; @@ -68,10 +68,10 @@ virshCheckpointNameCompleter(vshControl *ctl, error: for (; i < ncheckpoints; i++) virshDomainCheckpointFree(checkpoints[i]); -VIR_FREE(checkpoints); +g_free(checkpoints); for (i = 0; i < ncheckpoints; i++) -VIR_FREE(ret[i]); -VIR_FREE(ret); +g_free(ret[i]); +g_free(ret); virshDomainFree(dom); return NULL; } diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 6bd09dcb0f..8159a0a58d 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -75,7 +75,7 @@ virshDomainNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < ndomains; i++) virshDomainFree(domains[i]); -VIR_FREE(domains); +g_free(domains); return ret; } @@ -127,7 +127,7 @@ virshDomainUUIDCompleter(vshControl *ctl, cleanup: for (i = 0; i < ndomains; i++) virshDomainFree(domains[i]); -VIR_FREE(domains); +g_free(domains); return ret; } diff --git a/tools/virsh-completer-interface.c b/tools/virsh-completer-interface.c index 417374322a..6ac11a402f 100644 --- a/tools/virsh-completer-interface.c +++ b/tools/virsh-completer-interface.c @@ -61,6 +61,6 @@ virshInterfaceNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < nifaces; i++) virInterfaceFree(ifaces[i]); -VIR_FREE(ifaces); +g_free(ifaces); return ret; } diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c index c215e27720..c29d367c76 100644 --- a/tools/virsh-completer-network.c +++ b/tools/virsh-completer-network.c @@ -63,7 +63,7 @@ virshNetworkNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < nnets; i++) virNetworkFree(nets[i]); -VIR_FREE(nets); +g_free(nets); return ret; } @@ -124,17 +124,17 @@ virshNetworkPortUUIDCompleter(vshControl *ctl, virNetworkPortFree(ports[i]); } -VIR_FREE(ports); +g_free(ports); return ret; error: for (; i < nports; i++) virNetworkPortFree(ports[i]); -VIR_FREE(ports); +g_free(ports); for (i = 0; i < nports; i++) -VIR_FREE(ret[i]); -VIR_FREE(ret); +g_free(ret[i]); +g_free(ret); return NULL; } @@ -176,6 +176,6 @@ virshNetworkUUIDCompleter(vshControl *ctl, cleanup: for (i = 0; i < nnets; i++) virNetworkFree(nets[i]); -VIR_FREE(nets); +g_free(nets); return ret; } diff --git a/tools/virsh-completer-nodedev.c b/tools/virsh-completer-nodedev.c index 5425f11262..94da5965f2 100644 --- a/tools/virsh-completer-nodedev.c +++ b/tools/virsh-completer-nodedev.c @@ -61,7 +61,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < ndevs; i++) virNodeDeviceFree(devs[i]); -VIR_FREE(devs); +g_free(devs); return ret; } diff --git a/tools/virsh-completer-nwfilter.c b/tools/virsh-completer-nwfilter.c index 989a363847..a3fbeded29 100644 --- a/tools/virsh-completer-nwfilter.c +++ b/tools/virsh-completer-nwfilter.c @@ -59,7 +59,7 @@ virshNWFilterNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < nnwfilters; i++) virNWFilterFree(nwfilters[i]); -VIR_FREE(nwfilters); +g_free(nwfilters); return ret; } @@ -98,6 +98,6 @@ virshNWFilterBindingNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < nbindings; i++) virNWFilterBindingFree(bindings[i]); -VIR_FREE(bindings); +g_free(bindings); return ret; } diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c index ed3b1e35ff..35eeed6043 100644 --- a/tools/virsh-completer-pool.c +++ b/tools/virsh-completer-pool.c @@ -64,7 +64,7 @@ virshStoragePoolNameCompleter(vshControl *ctl, cleanup: for (i = 0; i < npools; i++) virStoragePoolFree(pools[i]); -VIR_FREE(pools); +g_free(pools); return ret; } diff --git a/tools/virsh-completer
[libvirt PATCH 1/3] virsh-completer: use g_clear_pointer in virshCommaStringListComplete
Signed-off-by: Ján Tomko --- tools/virsh-completer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 1d9d212f8a..bb6550ee63 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -105,7 +105,7 @@ virshCommaStringListComplete(const char *input, if ((comma = strrchr(inputCopy, ','))) *comma = '\0'; else -VIR_FREE(inputCopy); +g_clear_pointer(&inputCopy, g_free); } if (inputCopy && !(inputList = virStringSplit(inputCopy, ",", 0))) -- 2.26.2
[libvirt PATCH 0/3] virsh-completer: Use Glib memory functions (glib chronicles)
Ján Tomko (3): virsh-completer: use g_clear_pointer in virshCommaStringListComplete virsh-completer: use g_free instead of VIR_FREE virsh-completer: use g_new0 instead of VIR_ALLOC_N tools/virsh-completer-checkpoint.c | 11 tools/virsh-completer-domain.c | 44 ++ tools/virsh-completer-host.c | 6 ++-- tools/virsh-completer-interface.c | 6 ++-- tools/virsh-completer-network.c| 25 +++-- tools/virsh-completer-nodedev.c| 12 +++- tools/virsh-completer-nwfilter.c | 12 +++- tools/virsh-completer-pool.c | 12 +++- tools/virsh-completer-secret.c | 8 ++ tools/virsh-completer-snapshot.c | 5 ++-- tools/virsh-completer-volume.c | 5 ++-- tools/virsh-completer.c| 5 ++-- 12 files changed, 55 insertions(+), 96 deletions(-) -- 2.26.2
Re: [PATCH 0/2] qemu: migration corner case fix and cleanup
On 8/18/20 12:26 PM, Nikolay Shirokovskiy wrote: Nikolay Shirokovskiy (2): qemu: fix qemuMigrationSrcCleanup to use qemuMigrationJobFinish qemu: don't needlessly unset close callback during perform phase src/qemu/qemu_migration.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) Patches look good to me, but since it was Jirka who added the call you're removing in 2/2 I'll let him share opinion too. Reviewed-by: Michal Privoznik Michal
Re: [libvirt PATCH 2/2] esx: implement domainInterfaceAddresses
On 9/14/20 11:15 AM, Pino Toscano wrote:
Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.
Signed-off-by: Pino Toscano
---
docs/drvesx.html.in | 4 ++
src/esx/esx_driver.c | 166 +++
2 files changed, 170 insertions(+)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 38de640c2a..3acb7e5c51 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -799,6 +799,10 @@ Enter administrator password for example-vcenter.com:
virDomainGetHostname
+
+virDomainInterfaceAddresses (only for the
+VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+
virDomainReboot
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index bddc588977..0f63b5db4d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,171 @@ esxDomainGetHostname(virDomainPtr domain,
}
+static int
+esxParseIPAddress(const char *ipAddress, int prefixLength,
+ virDomainIPAddress *addr)
+{
+virSocketAddr tmp_addr;
+virIPAddrType addr_type;
+
+if (virSocketAddrParseAny(&tmp_addr, ipAddress, AF_UNSPEC, false) <= 0)
+return 0;
+
+switch (VIR_SOCKET_ADDR_FAMILY(&tmp_addr)) {
+case AF_INET:
+addr_type = VIR_IP_ADDR_TYPE_IPV4;
+break;
+case AF_INET6:
+addr_type = VIR_IP_ADDR_TYPE_IPV6;
+break;
+default:
+return 0;
+}
+
+addr->type = addr_type;
+addr->addr = g_strdup(ipAddress);
+addr->prefix = prefixLength;
+
+return 1;
+}
+
+
+static int
+esxDomainInterfaceAddresses(virDomainPtr domain,
+virDomainInterfacePtr **ifaces,
+unsigned int source,
+unsigned int flags)
+{
+int result = -1;
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+esxVI_DynamicProperty *dynamicProperty;
+esxVI_GuestNicInfo *guestNicInfoList = NULL;
+esxVI_GuestNicInfo *guestNicInfo = NULL;
+virDomainInterfacePtr *ifaces_ret = NULL;
+size_t ifaces_count = 0;
+size_t i;
+int ret;
+
+virCheckFlags(0, -1);
+if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Unknown IP address data source %d"),
+ source);
+return -1;
+}
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+if (esxVI_String_AppendValueListToList(&propertyNameList,
+ "runtime.powerState\0"
+ "guest.net") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, &virtualMachine,
+ esxVI_Occurrence_RequiredItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not powered on"));
+goto cleanup;
+}
+
+for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "guest.net")) {
+if (esxVI_GuestNicInfo_CastListFromAnyType
+ (dynamicProperty->val, &guestNicInfoList) < 0) {
+goto cleanup;
+}
+}
+}
+
+if (!guestNicInfoList)
+goto cleanup;
This looks suspicious. If I understand the code correctly then this
means we haven't found any network config. What we usually do is we
return an empty array (*ifaces = NULL) and return 0. But if this is a
more serious error then we need a virReportError() here.
+
+for (guestNicInfo = guestNicInfoList; guestNicInfo;
+ guestNicInfo = guestNicInfo->_next) {
+virDomainInterfacePtr iface = NULL;
+size_t addrs_count = 0;
+
+if (guestNicInfo->connected != esxVI_Boolean_True ||
+!guestNicInfo->network) {
+continue;
+}
+
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface->naddrs = 0;
+iface->name = g_strdup(guestNicInfo->network);
+iface->hwaddr = g_strdup(guestNicInfo->macAddress);
+
+if (guestNicInfo->ipConfig) {
+esxVI_NetIpConfigInfoIpAddress *ipAddress;
+for (ipAddress = guestNicInf
Re: [libvirt PATCH v2 0/7] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.
On a Monday in 2020, Tim Wiederhake wrote: V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00633.html Tim Wiederhake (7): tests: Fix false positive in testConfRoundTrip. util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET src/util/viralloc.h | 29 - src/util/virerror.c | 15 --- src/util/virjson.c | 3 +-- tests/virconftest.c | 28 +--- 4 files changed, 14 insertions(+), 61 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v2 1/7] tests: Fix false positive in testConfRoundTrip.
On a Monday in 2020, Tim Wiederhake wrote:
testConfRoundTrip would return 0 (success) if virConfWriteMem returned 0
(nothing
written) and virTestCompareToFile failed.
s/returned 0/succeeded/
The caller treats all non-negative values as a success. (as is usual in
the codebase)
Jano
Signed-off-by: Tim Wiederhake
---
tests/virconftest.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c
index ab29b5b712..cac3718495 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -51,8 +51,7 @@ static int testConfRoundTrip(const void *opaque)
fprintf(stderr, "Failed to process %s\n", srcfile);
goto cleanup;
}
-ret = virConfWriteMem(buffer, &len, conf);
-if (ret < 0) {
+if (virConfWriteMem(buffer, &len, conf) < 0) {
fprintf(stderr, "Failed to serialize %s back\n", srcfile);
goto cleanup;
}
--
2.26.2
signature.asc
Description: PGP signature
Re: [libvirt PATCH] esx: implement connectListAllNetworks
On 9/14/20 11:10 AM, Pino Toscano wrote:
Implement the .connectListAllNetworks networks API in the esx network
driver.
Signed-off-by: Pino Toscano
---
src/esx/esx_network_driver.c | 64
1 file changed, 64 insertions(+)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 88843aae2f..5d9c1e3c2c 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+ virNetworkPtr **nets,
+ unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+ &hostVirtualSwitchList) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch);
+
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
The virConnectListAllNetworks() documents that @nets can be NULL if the
caller is interested only in the count of networks. I think that direct
dereference will lead to a crash. I think we want this body look a bit
different then:
if (nets) {
if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
goto cleanup;
} else {
count = ++count;
}
+goto cleanup;
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*nets) {
And this needs to be: if (nets && *nets) ...
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free(&hostVirtualSwitchList);
+
+return ret;
+}
+#undef MATCH
+
+
+
virNetworkDriver esxNetworkDriver = {
.connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
.connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +940,5 @@ virNetworkDriver esxNetworkDriver = {
.networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
.networkIsActive = esxNetworkIsActive, /* 0.10.0 */
.networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.8.0 */
};
Reviewed-by: Michal Privoznik
Michal
Re: [libvirt PATCH 0/3] fix a memory leak on qemuProcessReconnect
On 9/14/20 1:49 PM, Ján Tomko wrote: The first two patches have no functional impact on current code. Ján Tomko (3): qemu: rename qemuDomainObjFreeJob -> qemuDomainObjClearJob qemu: qemuDomainObjClearJob: use g_clear_pointer qemuProcessReconnect: clear 'oldjob' src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_domainjob.c | 7 +-- src/qemu/qemu_domainjob.h | 3 ++- src/qemu/qemu_process.c | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH v2] cphp: remove deprecated cpu-add command(s)
On Mon, 14 Sep 2020 03:46:14 -0400 Igor Mammedov wrote: > theses were deprecated since 4.0, remove both HMP and QMP variants. s/theses/These/ > > Users should use device_add command instead. To get list of > possible CPUs and options, use 'info hotpluggable-cpus' HMP > or query-hotpluggable-cpus QMP command. > > Signed-off-by: Igor Mammedov > Reviewed-by: Thomas Huth > Acked-by: Dr. David Alan Gilbert > --- > include/hw/boards.h | 1 - > include/hw/i386/pc.h| 1 - > include/monitor/hmp.h | 1 - > docs/system/deprecated.rst | 25 + > hmp-commands.hx | 15 -- > hw/core/machine-hmp-cmds.c | 12 - > hw/core/machine-qmp-cmds.c | 12 - > hw/i386/pc.c| 27 -- > hw/i386/pc_piix.c | 1 - > hw/s390x/s390-virtio-ccw.c | 12 - > qapi/machine.json | 24 - > tests/qtest/cpu-plug-test.c | 100 > tests/qtest/test-hmp.c | 1 - > 13 files changed, 21 insertions(+), 211 deletions(-) Acked-by: Cornelia Huck
Re: device compatibility interface for live migration with assigned devices
On Mon, 14 Sep 2020 13:48:43 + "Zeng, Xin" wrote: > On Saturday, September 12, 2020 12:52 AM > Alex Williamson wrote: > > To: Zhao, Yan Y > > Cc: Sean Mooney ; Cornelia Huck > > ; Daniel P.Berrangé ; > > k...@vger.kernel.org; libvir-list@redhat.com; Jason Wang > > ; qemu-de...@nongnu.org; > > kwankh...@nvidia.com; eau...@redhat.com; Wang, Xin-ran > ran.w...@intel.com>; cor...@lwn.net; openstack- > > disc...@lists.openstack.org; Feng, Shaohe ; Tian, > > Kevin ; Parav Pandit ; Ding, > > Jian-feng ; dgilb...@redhat.com; > > zhen...@linux.intel.com; Xu, Hejie ; > > bao.yum...@zte.com.cn; intel-gvt-...@lists.freedesktop.org; > > eskul...@redhat.com; Jiri Pirko ; dinec...@redhat.com; > > de...@ovirt.org > > Subject: Re: device compatibility interface for live migration with assigned > > devices > > > > On Fri, 11 Sep 2020 08:56:00 +0800 > > Yan Zhao wrote: > > > > > On Thu, Sep 10, 2020 at 12:02:44PM -0600, Alex Williamson wrote: > > > > On Thu, 10 Sep 2020 13:50:11 +0100 > > > > Sean Mooney wrote: > > > > > > > > > On Thu, 2020-09-10 at 14:38 +0200, Cornelia Huck wrote: > > > > > > On Wed, 9 Sep 2020 10:13:09 +0800 > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > still, I'd like to put it more explicitly to make ensure it's > > > > > > > > > not > > missed: > > > > > > > > > the reason we want to specify compatible_type as a trait and > > check > > > > > > > > > whether target compatible_type is the superset of source > > > > > > > > > compatible_type is for the consideration of backward > > compatibility. > > > > > > > > > e.g. > > > > > > > > > an old generation device may have a mdev type xxx-v4-yyy, > > while a newer > > > > > > > > > generation device may be of mdev type xxx-v5-yyy. > > > > > > > > > with the compatible_type traits, the old generation device is > > > > > > > > > still > > > > > > > > > able to be regarded as compatible to newer generation device > > even their > > > > > > > > > mdev types are not equal. > > > > > > > > > > > > > > > > If you want to support migration from v4 to v5, can't the > > (presumably > > > > > > > > newer) driver that supports v5 simply register the v4 type as > > > > > > > > well, > > so > > > > > > > > that the mdev can be created as v4? (Just like QEMU versioned > > machine > > > > > > > > types work.) > > > > > > > > > > > > > > yes, it should work in some conditions. > > > > > > > but it may not be that good in some cases when v5 and v4 in the > > name string > > > > > > > of mdev type identify hardware generation (e.g. v4 for gen8, and > > > > > > > v5 > > for > > > > > > > gen9) > > > > > > > > > > > > > > e.g. > > > > > > > (1). when src mdev type is v4 and target mdev type is v5 as > > > > > > > software does not support it initially, and v4 and v5 identify > > hardware > > > > > > > differences. > > > > > > > > > > > > My first hunch here is: Don't introduce types that may be compatible > > > > > > later. Either make them compatible, or make them distinct by design, > > > > > > and possibly add a different, compatible type later. > > > > > > > > > > > > > then after software upgrade, v5 is now compatible to v4, should > > > > > > > the > > > > > > > software now downgrade mdev type from v5 to v4? > > > > > > > not sure if moving hardware generation info into a separate > > attribute > > > > > > > from mdev type name is better. e.g. remove v4, v5 in mdev type, > > while use > > > > > > > compatible_pci_ids to identify compatibility. > > > > > > > > > > > > If the generations are compatible, don't mention it in the mdev > > > > > > type. > > > > > > If they aren't, use distinct types, so that management software > > doesn't > > > > > > have to guess. At least that would be my naive approach here. > > > > > yep that is what i would prefer to see too. > > > > > > > > > > > > > > > > > > > > (2) name string of mdev type is composed by "driver_name + > > type_name". > > > > > > > in some devices, e.g. qat, different generations of devices are > > binding to > > > > > > > drivers of different names, e.g. "qat-v4", "qat-v5". > > > > > > > then though type_name is equal, mdev type is not equal. e.g. > > > > > > > "qat-v4-type1", "qat-v5-type1". > > > > > > > > > > > > I guess that shows a shortcoming of that "driver_name + type_name" > > > > > > approach? Or maybe I'm just confused. > > > > > yes i really dont like haveing the version in the mdev-type name > > > > > i would stongly perfger just qat-type-1 wehere qat is just there as a > > > > > way > > of namespacing. > > > > > although symmetric-cryto, asymmetric-cryto and compression woudl > > be a better name then type-1, type-2, type-3 if > > > > > that is what they would end up mapping too. e.g. qat-compression or > > qat-aes is a much better name then type-1 > > > > > higher layers of software are unlikely to parse the mdev names but as > > > > > a > > human looking at
Re: [PATCH] docs/system: clarify deprecation scheduled
On Mon, 14 Sep 2020 at 15:22, Daniel P. Berrangé wrote: > So we're changing > > The feature will remain functional for 2 releases prior to actual removal. > > to > > The feature will remain functional for 1 more release after deprecation. > > How about > > The feature will remain functional for the release in which it was > deprecated and one further release. After these two releases, the > feature is liable to be removed. I think the thing which tends to confuse me about the wording is that it's phrased in terms of "releases", ie point events, (which is OK for users) but the developers who are adding deprecation notices and then removing features probably think more in terms of "release cycles" (ie the periods of time between the point events), or at least I do, so I have to mentally convert "functional for two releases" into "so if I deprecate it in this cycle, then I have to leave the code present in the next cycle and then am OK to delete the code the cycle after that". But I don't have any good suggestions for wording, and your proposed text is definitely clearer I think. -- PMM
Re: [PATCH] docs/system: clarify deprecation scheduled
On 9/14/20 9:21 AM, Daniel P. Berrangé wrote: On Tue, Aug 11, 2020 at 11:47:36AM +0100, Stefan Hajnoczi wrote: The sentence explaining the deprecation schedule is ambiguous. Make it clear that a feature deprecated in the Nth release is guaranteed to remain available in the N+1th release. Removal can occur in the N+2nd release or later. So we're changing The feature will remain functional for 2 releases prior to actual removal. to The feature will remain functional for 1 more release after deprecation. How about The feature will remain functional for the release in which it was deprecated and one further release. After these two releases, the feature is liable to be removed. Longer, but definitely conveys more information in an easier-to-understand format. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] docs/system: clarify deprecation scheduled
On Tue, Aug 11, 2020 at 11:47:36AM +0100, Stefan Hajnoczi wrote:
> The sentence explaining the deprecation schedule is ambiguous. Make it
> clear that a feature deprecated in the Nth release is guaranteed to
> remain available in the N+1th release. Removal can occur in the N+2nd
> release or later.
>
> As an example of this in action, see commit
> 25956af3fe5dd0385ad8017bc768a6afe41e2a74 ("block: Finish deprecation of
> 'qemu-img convert -n -o'"). The feature was deprecated in QEMU 4.2.0. It
> was present in the 5.0.0 release and removed in the 5.1.0 release.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> docs/system/deprecated.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 851dbdeb8a..fecfb2f1c1 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -4,9 +4,9 @@ Deprecated features
> In general features are intended to be supported indefinitely once
> introduced into QEMU. In the event that a feature needs to be removed,
> it will be listed in this section. The feature will remain functional
> -for 2 releases prior to actual removal. Deprecated features may also
> -generate warnings on the console when QEMU starts up, or if activated
> -via a monitor command, however, this is not a mandatory requirement.
> +for 1 more release after deprecation. Deprecated features may also generate
> +warnings on the console when QEMU starts up, or if activated via a monitor
> +command, however, this is not a mandatory requirement.
So we're changing
The feature will remain functional for 2 releases prior to actual removal.
to
The feature will remain functional for 1 more release after deprecation.
How about
The feature will remain functional for the release in which it was
deprecated and one further release. After these two releases, the
feature is liable to be removed.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] docs/system: clarify deprecation scheduled
On Tue, Aug 11, 2020 at 11:47:36AM +0100, Stefan Hajnoczi wrote:
> The sentence explaining the deprecation schedule is ambiguous. Make it
> clear that a feature deprecated in the Nth release is guaranteed to
> remain available in the N+1th release. Removal can occur in the N+2nd
> release or later.
>
> As an example of this in action, see commit
> 25956af3fe5dd0385ad8017bc768a6afe41e2a74 ("block: Finish deprecation of
> 'qemu-img convert -n -o'"). The feature was deprecated in QEMU 4.2.0. It
> was present in the 5.0.0 release and removed in the 5.1.0 release.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> docs/system/deprecated.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Ping?
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 851dbdeb8a..fecfb2f1c1 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -4,9 +4,9 @@ Deprecated features
> In general features are intended to be supported indefinitely once
> introduced into QEMU. In the event that a feature needs to be removed,
> it will be listed in this section. The feature will remain functional
> -for 2 releases prior to actual removal. Deprecated features may also
> -generate warnings on the console when QEMU starts up, or if activated
> -via a monitor command, however, this is not a mandatory requirement.
> +for 1 more release after deprecation. Deprecated features may also generate
> +warnings on the console when QEMU starts up, or if activated via a monitor
> +command, however, this is not a mandatory requirement.
>
> Prior to the 2.10.0 release there was no official policy on how
> long features would be deprecated prior to their removal, nor
> --
> 2.26.2
>
signature.asc
Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 03:48:31PM +0200, Erik Skultety wrote: > On Mon, Sep 14, 2020 at 04:41:59PM +0300, Eli Cohen wrote: > > On Mon, Sep 14, 2020 at 02:34:51PM +0100, Daniel P. Berrangé wrote: > > > On Mon, Sep 14, 2020 at 03:30:15PM +0200, Erik Skultety wrote: > > > > On Mon, Sep 14, 2020 at 04:24:20PM +0300, Eli Cohen wrote: > > > > > On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > > > > > > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > > > > > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé > > > > > > > wrote: > > > > > > > > > > > > > > > > > > $ ninja -C build > > > > > > > > > ninja: Entering directory `build' > > > > > > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py > > > > > > > > > custom command > > > > > > > > > FAILED: docs/manpages/virsh.html.in > > > > > > > > > /usr/bin/meson --internal exe --capture > > > > > > > > > docs/manpages/virsh.html.in -- > > > > > > > > > /usr/local/bin/rst2html5 --stylesheet= --strict > > > > > > > > > docs/manpages/virsh.rst > > > > > > > > > > > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke > > > > > > > > you've > > > > > > > > installed a non-standard docutils build / version and that > > > > > > > > appears to > > > > > > > > be breaking docs generation. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > > > > > > /usr/local/bin/. In anycase I did not put any package in > > > > > > > /usr/local. In > > > > > > > any case, I see different versions of rst2html5 in both /usr/bin > > > > > > > and > > > > > > > /usr/local/bin. > > > > > > > > > > > > It's not the build script that looks under /usr/local/bin, it's the > > > > > > PATH > > > > > > variable which likely lists /usr/local/bin before /usr/bin. So, the > > > > > > only > > > > > > explanation I have for you as for how rst2html5 appeared under > > > > > > /usr/local/bin > > > > > > is that you or someone else installed it with pip as root. > > > > > > And the new docutils 0.16 looks like broke libvirt, so please stick > > > > > > with 0.15. > > > > > > > > > > > I am on 0.15 > > > > > > > > > > I tried all the recommendations given but I have htting issues all the > > > > > time. > > > > > > > > > > If anyone has it working over any Linux distribution, I am will to try > > > > > that. I have a server ready for these experiments. I just need to have > > > > > the latest libvirt running on my system. > > > > > > > > > > > > > Well, if you look at our CI pipelines [1], you'll see that apart from > > > > Rawhide, > > > > everything is green, so clearly the builds work. What other issues do > > > > you see? > > > > Look at the Dockerfiles for the distros we have, install the packages > > > > you see > > > > in the list and try re-running meson. > > > > > > That won't be enough - the bad rst5html5 install in /usr/local/ needs to > > > be eliminated, either by deleting it, or removing it from $PATH. > > > > > > > I already tried but it causes ninja to fail because it insists on > > getting rst2html5 from user local. See below: > > > > $ ninja -C build > > ninja: Entering directory `build' > > ninja: error: '/usr/local/bin/rst2html5', needed by > > 'docs/advanced-tests.html.p/advanced-tests.html.in', missing and no > > known rule to make it > > You need to reconfigure meson again...$ meson --reconfigure build > otherwise ninja will use the static paths determined by the first meson run. > That does it. Thanks a lot all you who helped. > Erik >
Re: [libvirt] [PATCH 00/17] virsh bash completion improvements
On 9/11/20 9:13 AM, Lin Ma wrote:
Lin Ma (17):
virshDomainNameCompleter: Add some of VIR_CONNECT_LIST_DOMAINS_ flags
virsh: snapshot: Only return domains that have snapshot images
virsh: managedsave-*: Only return domains that have a managed save
image
virsh: checkpoint: Only return domains that have checkpoints
virsh: Add event completer to --enable/--disable args of perf command
vshReadlineParse: Ignore vshReadlineOptionsGenerator for VSH_OT_INT
options
virsh: Add network uuid completion to network-name command
virsh: Add domain uuid completion to domname command
virsh: Add iothread IDs completion to iothread* commands
virsh: Add completion for --iothread argument of attach-disk command
virsh: Add vcpu IDs completion to vcpupin command
virsh: Add vcpu list completion to setvcpu command
virsh: Add logical CPU list completion for --cpulist argument
virsh: limit completion of net-dhcp-leases to active networks
virsh: limit completion of net-port* to active networks
virsh: limit completion of backup-{begin, dumpxml} to active domains
virsh: limit completion of checkpoint-{create, delete} to active
domains
tools/virsh-backup.c| 4 +-
tools/virsh-checkpoint.c| 17 +-
tools/virsh-completer-domain.c | 264 +++-
tools/virsh-completer-domain.h | 31
tools/virsh-completer-network.c | 42 +
tools/virsh-completer-network.h | 4 +
tools/virsh-domain.c| 33 +++-
tools/virsh-network.c | 12 +-
tools/virsh-snapshot.c | 16 +-
tools/vsh.c | 1 +
10 files changed, 395 insertions(+), 29 deletions(-)
Reviewed-by: Michal Privoznik
Michal
Re: [libvirt] [PATCH 07/17] virsh: Add network uuid completion to network-name command
On 9/11/20 9:13 AM, Lin Ma wrote:
Signed-off-by: Lin Ma
---
tools/virsh-completer-network.c | 42 +
tools/virsh-completer-network.h | 4
tools/virsh-network.c | 2 ++
3 files changed, 48 insertions(+)
diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c
index 8f0048ed6f..22ab4a80c3 100644
--- a/tools/virsh-completer-network.c
+++ b/tools/virsh-completer-network.c
@@ -137,3 +137,45 @@ virshNetworkPortUUIDCompleter(vshControl *ctl,
VIR_FREE(ret);
return NULL;
}
+
+
+char **
+virshNetworkUUIDCompleter(vshControl *ctl,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+virNetworkPtr *nets = NULL;
+int nnets = 0;
+size_t i = 0;
+char **ret = NULL;
+VIR_AUTOSTRINGLIST tmp = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if ((nnets = virConnectListAllNetworks(priv->conn, &nets, flags)) < 0)
+return NULL;
+
+if (VIR_ALLOC_N(tmp, nnets + 1) < 0)
+goto cleanup;
+
+for (i = 0; i < nnets; i++) {
+char uuid[VIR_UUID_STRING_BUFLEN];
+if (virNetworkGetUUIDString(nets[i], uuid) < 0) {
+vshError(ctl, "%s", _("Failed to get network's UUID"));
We don't like completers to report any kind of error because that would
harm the user experience. For instance it doesn't print the error on an
empty line:
virsh # net-name --network error: Failed to get ...
It's acceptable if completer returns nothing when failing.
+goto cleanup;
+}
+tmp[i] = g_strdup(uuid);
+}
+
+ret = g_steal_pointer(&tmp);
+
+ cleanup:
+for (i = 0; i < nnets; i++)
+virNetworkFree(nets[i]);
+VIR_FREE(nets);
+return ret;
+}
diff --git a/tools/virsh-completer-network.h b/tools/virsh-completer-network.h
index e317e483c1..8910e4525c 100644
--- a/tools/virsh-completer-network.h
+++ b/tools/virsh-completer-network.h
@@ -33,3 +33,7 @@ char ** virshNetworkEventNameCompleter(vshControl *ctl,
char ** virshNetworkPortUUIDCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char ** virshNetworkUUIDCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index f0f5358625..f488e840ac 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -806,6 +806,8 @@ static const vshCmdOptDef opts_network_name[] = {
{.name = "network",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
+ .completer = virshNetworkUUIDCompleter,
+ .completer_flags = 0,
This is not needed. flags are zero by default.
.help = N_("network uuid")
},
{.name = NULL}
I see you used the same pattern for next patches - the same comment
applies to them.
Michal
Re: [libvirt] [PATCH 05/17] virsh: Add event completer to --enable/--disable args of perf command
On 9/11/20 9:13 AM, Lin Ma wrote:
Signed-off-by: Lin Ma
---
tools/virsh-completer-domain.c | 49 ++
tools/virsh-completer-domain.h | 8 ++
tools/virsh-domain.c | 2 ++
3 files changed, 59 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 4c1261b06b..122a9d5f66 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -29,6 +29,7 @@
#include "virsh.h"
#include "virstring.h"
#include "virxml.h"
+#include "conf/domain_conf.h"
Including domain_conf.h looks like too much. IIUC, you need
virPerfEventTypeToString() prototype - that lives in virperf.h so
including just that should be fine.
char **
virshDomainNameCompleter(vshControl *ctl,
@@ -338,3 +339,51 @@ virshDomainHostnameSourceCompleter(vshControl *ctl
G_GNUC_UNUSED,
return ret;
}
+
+
+char **
+virshDomainPerfEnableCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags)
+{
+size_t i = 0;
+VIR_AUTOSTRINGLIST events = NULL;
+const char *event = NULL;
+
+virCheckFlags(0, NULL);
+
+if (VIR_ALLOC_N(events, VIR_PERF_EVENT_LAST + 1) < 0)
+return NULL;
+
+for (i = 0; i < VIR_PERF_EVENT_LAST; i++)
+events[i] = g_strdup(virPerfEventTypeToString(i));
+
+if (vshCommandOptStringQuiet(ctl, cmd, "enable", &event) < 0)
+return NULL;
+
+return virshCommaStringListComplete(event, (const char **)events);
+}
+
+
+char **
+virshDomainPerfDisableCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags)
+{
+size_t i = 0;
+VIR_AUTOSTRINGLIST events = NULL;
+const char *event = NULL;
+
+virCheckFlags(0, NULL);
+
+if (VIR_ALLOC_N(events, VIR_PERF_EVENT_LAST + 1) < 0)
+return NULL;
+
+for (i = 0; i < VIR_PERF_EVENT_LAST; i++)
+events[i] = g_strdup(virPerfEventTypeToString(i));
+
+if (vshCommandOptStringQuiet(ctl, cmd, "disable", &event) < 0)
+return NULL;
+
+return virshCommaStringListComplete(event, (const char **)events);
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index b00b05e3bd..e3375c3c65 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -62,3 +62,11 @@ virshDomainInterfaceAddrSourceCompleter(vshControl *ctl,
char ** virshDomainHostnameSourceCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char ** virshDomainPerfEnableCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
+
+char ** virshDomainPerfDisableCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 68600d728a..1ba536466a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9333,10 +9333,12 @@ static const vshCmdOptDef opts_perf[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "enable",
.type = VSH_OT_STRING,
+ .completer = virshDomainPerfEnableCompleter,
.help = N_("perf events which will be enabled")
},
{.name = "disable",
.type = VSH_OT_STRING,
+ .completer = virshDomainPerfDisableCompleter,
.help = N_("perf events which will be disabled")
},
VIRSH_COMMON_OPT_DOMAIN_CONFIG,
Michal
RE: device compatibility interface for live migration with assigned devices
On Saturday, September 12, 2020 12:52 AM Alex Williamson wrote: > To: Zhao, Yan Y > Cc: Sean Mooney ; Cornelia Huck > ; Daniel P.Berrangé ; > k...@vger.kernel.org; libvir-list@redhat.com; Jason Wang > ; qemu-de...@nongnu.org; > kwankh...@nvidia.com; eau...@redhat.com; Wang, Xin-ran ran.w...@intel.com>; cor...@lwn.net; openstack- > disc...@lists.openstack.org; Feng, Shaohe ; Tian, > Kevin ; Parav Pandit ; Ding, > Jian-feng ; dgilb...@redhat.com; > zhen...@linux.intel.com; Xu, Hejie ; > bao.yum...@zte.com.cn; intel-gvt-...@lists.freedesktop.org; > eskul...@redhat.com; Jiri Pirko ; dinec...@redhat.com; > de...@ovirt.org > Subject: Re: device compatibility interface for live migration with assigned > devices > > On Fri, 11 Sep 2020 08:56:00 +0800 > Yan Zhao wrote: > > > On Thu, Sep 10, 2020 at 12:02:44PM -0600, Alex Williamson wrote: > > > On Thu, 10 Sep 2020 13:50:11 +0100 > > > Sean Mooney wrote: > > > > > > > On Thu, 2020-09-10 at 14:38 +0200, Cornelia Huck wrote: > > > > > On Wed, 9 Sep 2020 10:13:09 +0800 > > > > > Yan Zhao wrote: > > > > > > > > > > > > > still, I'd like to put it more explicitly to make ensure it's > > > > > > > > not > missed: > > > > > > > > the reason we want to specify compatible_type as a trait and > check > > > > > > > > whether target compatible_type is the superset of source > > > > > > > > compatible_type is for the consideration of backward > compatibility. > > > > > > > > e.g. > > > > > > > > an old generation device may have a mdev type xxx-v4-yyy, > while a newer > > > > > > > > generation device may be of mdev type xxx-v5-yyy. > > > > > > > > with the compatible_type traits, the old generation device is > > > > > > > > still > > > > > > > > able to be regarded as compatible to newer generation device > even their > > > > > > > > mdev types are not equal. > > > > > > > > > > > > > > If you want to support migration from v4 to v5, can't the > (presumably > > > > > > > newer) driver that supports v5 simply register the v4 type as > > > > > > > well, > so > > > > > > > that the mdev can be created as v4? (Just like QEMU versioned > machine > > > > > > > types work.) > > > > > > > > > > > > yes, it should work in some conditions. > > > > > > but it may not be that good in some cases when v5 and v4 in the > name string > > > > > > of mdev type identify hardware generation (e.g. v4 for gen8, and v5 > for > > > > > > gen9) > > > > > > > > > > > > e.g. > > > > > > (1). when src mdev type is v4 and target mdev type is v5 as > > > > > > software does not support it initially, and v4 and v5 identify > hardware > > > > > > differences. > > > > > > > > > > My first hunch here is: Don't introduce types that may be compatible > > > > > later. Either make them compatible, or make them distinct by design, > > > > > and possibly add a different, compatible type later. > > > > > > > > > > > then after software upgrade, v5 is now compatible to v4, should the > > > > > > software now downgrade mdev type from v5 to v4? > > > > > > not sure if moving hardware generation info into a separate > attribute > > > > > > from mdev type name is better. e.g. remove v4, v5 in mdev type, > while use > > > > > > compatible_pci_ids to identify compatibility. > > > > > > > > > > If the generations are compatible, don't mention it in the mdev type. > > > > > If they aren't, use distinct types, so that management software > doesn't > > > > > have to guess. At least that would be my naive approach here. > > > > yep that is what i would prefer to see too. > > > > > > > > > > > > > > > > > (2) name string of mdev type is composed by "driver_name + > type_name". > > > > > > in some devices, e.g. qat, different generations of devices are > binding to > > > > > > drivers of different names, e.g. "qat-v4", "qat-v5". > > > > > > then though type_name is equal, mdev type is not equal. e.g. > > > > > > "qat-v4-type1", "qat-v5-type1". > > > > > > > > > > I guess that shows a shortcoming of that "driver_name + type_name" > > > > > approach? Or maybe I'm just confused. > > > > yes i really dont like haveing the version in the mdev-type name > > > > i would stongly perfger just qat-type-1 wehere qat is just there as a > > > > way > of namespacing. > > > > although symmetric-cryto, asymmetric-cryto and compression woudl > be a better name then type-1, type-2, type-3 if > > > > that is what they would end up mapping too. e.g. qat-compression or > qat-aes is a much better name then type-1 > > > > higher layers of software are unlikely to parse the mdev names but as a > human looking at them its much eaiser to > > > > understand if the names are meaningful. the qat prefix i think is > important however to make sure that your mdev-types > > > > dont colide with other vendeors mdev types. so i woudl encurage all > vendors to prefix there mdev types with etiher the > > > > device name or the vendor. > > > > > > +1 to all this, the mdev type is meant to indicate a software > > > compatible interface, if different hardwa
Re: [PATCH 17/17] qemuxml2argvtest: hostdev-scsi-virtio-scsi: Use longer user-alias for SCSI hostdev
On a Friday in 2020, Peter Krempa wrote: Test that we can cope with a long useralias when generating SCSI hostdev commandline. Signed-off-by: Peter Krempa --- .../hostdev-scsi-virtio-scsi.x86_64-2.8.0.args| 8 +--- .../hostdev-scsi-virtio-scsi.x86_64-4.1.0.args| 8 +--- .../hostdev-scsi-virtio-scsi.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml | 2 +- tests/qemuxml2xmloutdata/hostdev-scsi-virtio-scsi.xml | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 15/17] qemuDomainPrepareHostdev: base hostdev secret object names on backend alias
On a Friday in 2020, Peter Krempa wrote: The secret object is used to pass data to the backend so it's better fitting to base the secret object name on the SCSI host device backend name. Since we store the object alias in the status XML this modification is safe in regards to existing guests. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 5 - .../qemuxml2argvdata/hostdev-scsi-lsi.x86_64-latest.args | 8 .../hostdev-scsi-virtio-scsi.x86_64-latest.args | 8 3 files changed, 12 insertions(+), 9 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 16/17] qemuDomainPrepareHostdev: Don't base backend nodename on device alias
On a Friday in 2020, Peter Krempa wrote: QEMU's blockdev nodenames which are used to back SCSI/iSCSI hostdevs are limited to 32 characters. If a user passes a very long user alias as name of the host device it's easy to end up with a too-long nodename. To prevent this from happening don't base the nodename on the possibly user-specified alias but on the normal sequential node name generator. We then store the name in the status XML for further use. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 3 +- .../hostdev-scsi-lsi.x86_64-latest.args | 38 --- ...ostdev-scsi-virtio-scsi.x86_64-latest.args | 36 +- 3 files changed, 36 insertions(+), 41 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 04:41:59PM +0300, Eli Cohen wrote: > On Mon, Sep 14, 2020 at 02:34:51PM +0100, Daniel P. Berrangé wrote: > > On Mon, Sep 14, 2020 at 03:30:15PM +0200, Erik Skultety wrote: > > > On Mon, Sep 14, 2020 at 04:24:20PM +0300, Eli Cohen wrote: > > > > On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > > > > > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > > > > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > > > > > > > > > $ ninja -C build > > > > > > > > ninja: Entering directory `build' > > > > > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom > > > > > > > > command > > > > > > > > FAILED: docs/manpages/virsh.html.in > > > > > > > > /usr/bin/meson --internal exe --capture > > > > > > > > docs/manpages/virsh.html.in -- > > > > > > > > /usr/local/bin/rst2html5 --stylesheet= --strict > > > > > > > > docs/manpages/virsh.rst > > > > > > > > > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke > > > > > > > you've > > > > > > > installed a non-standard docutils build / version and that > > > > > > > appears to > > > > > > > be breaking docs generation. > > > > > > > > > > > > > > > > > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > > > > > /usr/local/bin/. In anycase I did not put any package in > > > > > > /usr/local. In > > > > > > any case, I see different versions of rst2html5 in both /usr/bin and > > > > > > /usr/local/bin. > > > > > > > > > > It's not the build script that looks under /usr/local/bin, it's the > > > > > PATH > > > > > variable which likely lists /usr/local/bin before /usr/bin. So, the > > > > > only > > > > > explanation I have for you as for how rst2html5 appeared under > > > > > /usr/local/bin > > > > > is that you or someone else installed it with pip as root. > > > > > And the new docutils 0.16 looks like broke libvirt, so please stick > > > > > with 0.15. > > > > > > > > > I am on 0.15 > > > > > > > > I tried all the recommendations given but I have htting issues all the > > > > time. > > > > > > > > If anyone has it working over any Linux distribution, I am will to try > > > > that. I have a server ready for these experiments. I just need to have > > > > the latest libvirt running on my system. > > > > > > > > > > Well, if you look at our CI pipelines [1], you'll see that apart from > > > Rawhide, > > > everything is green, so clearly the builds work. What other issues do you > > > see? > > > Look at the Dockerfiles for the distros we have, install the packages you > > > see > > > in the list and try re-running meson. > > > > That won't be enough - the bad rst5html5 install in /usr/local/ needs to > > be eliminated, either by deleting it, or removing it from $PATH. > > > > I already tried but it causes ninja to fail because it insists on > getting rst2html5 from user local. See below: > > $ ninja -C build > ninja: Entering directory `build' > ninja: error: '/usr/local/bin/rst2html5', needed by > 'docs/advanced-tests.html.p/advanced-tests.html.in', missing and no > known rule to make it You need to reconfigure meson again...$ meson --reconfigure build otherwise ninja will use the static paths determined by the first meson run. Erik
Re: [PATCH 14/17] qemuDomainPrepareHostdev: Allocate backend nodenames in the prepare function
On a Friday in 2020, Peter Krempa wrote: Allocate the nodename in the setup function rather than in the command line generator. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 7 --- src/qemu/qemu_domain.c | 8 2 files changed, 12 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 12/17] qemu: domain: Extract preparation of hostdev specific data to a separate function
On a Friday in 2020, Peter Krempa wrote: Historically we've prepared secrets for all objects in one place. This doesn't make much sense and it's semantically more appealing to prepare everything for a single device type in one place. Move the setup of the (iSCSI|SCSI) hostdev secrets into a new function which will be used to setup other things as well in the future. This is a similar approach we do for disks. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 59 - src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 21 +++ 4 files changed, 78 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 13/17] qemuDomainSecretHostdevPrepare: remove
On a Friday in 2020, Peter Krempa wrote: The function is no longer used once we setup per-hostdev data. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 50 -- src/qemu/qemu_domain.h | 4 2 files changed, 54 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 02:34:51PM +0100, Daniel P. Berrangé wrote: > On Mon, Sep 14, 2020 at 03:30:15PM +0200, Erik Skultety wrote: > > On Mon, Sep 14, 2020 at 04:24:20PM +0300, Eli Cohen wrote: > > > On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > > > > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > > > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > > > > > > > $ ninja -C build > > > > > > > ninja: Entering directory `build' > > > > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom > > > > > > > command > > > > > > > FAILED: docs/manpages/virsh.html.in > > > > > > > /usr/bin/meson --internal exe --capture > > > > > > > docs/manpages/virsh.html.in -- > > > > > > > /usr/local/bin/rst2html5 --stylesheet= --strict > > > > > > > docs/manpages/virsh.rst > > > > > > > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > > > > > installed a non-standard docutils build / version and that appears > > > > > > to > > > > > > be breaking docs generation. > > > > > > > > > > > > > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > > > > /usr/local/bin/. In anycase I did not put any package in /usr/local. > > > > > In > > > > > any case, I see different versions of rst2html5 in both /usr/bin and > > > > > /usr/local/bin. > > > > > > > > It's not the build script that looks under /usr/local/bin, it's the PATH > > > > variable which likely lists /usr/local/bin before /usr/bin. So, the only > > > > explanation I have for you as for how rst2html5 appeared under > > > > /usr/local/bin > > > > is that you or someone else installed it with pip as root. > > > > And the new docutils 0.16 looks like broke libvirt, so please stick > > > > with 0.15. > > > > > > > I am on 0.15 > > > > > > I tried all the recommendations given but I have htting issues all the > > > time. > > > > > > If anyone has it working over any Linux distribution, I am will to try > > > that. I have a server ready for these experiments. I just need to have > > > the latest libvirt running on my system. > > > > > > > Well, if you look at our CI pipelines [1], you'll see that apart from > > Rawhide, > > everything is green, so clearly the builds work. What other issues do you > > see? > > Look at the Dockerfiles for the distros we have, install the packages you > > see > > in the list and try re-running meson. > > That won't be enough - the bad rst5html5 install in /usr/local/ needs to > be eliminated, either by deleting it, or removing it from $PATH. > I already tried but it causes ninja to fail because it insists on getting rst2html5 from user local. See below: $ ninja -C build ninja: Entering directory `build' ninja: error: '/usr/local/bin/rst2html5', needed by 'docs/advanced-tests.html.p/advanced-tests.html.in', missing and no known rule to make it $ which rst2html5 /usr/bin/rst2html5 > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| >
Re: [PATCH 11/17] qemuBlockStorageSourceAttachData: remove 'storageNodeNameCopy'
On a Friday in 2020, Peter Krempa wrote: This was a hack when we were locally regenerating the nodename so that it's not leaked. Now that we use proper virStorageSource with persistence it's no longer required. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c | 1 - src/qemu/qemu_block.h | 1 - 2 files changed, 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 10/17] qemuBuildHostdevSCSI(A|De)tachPrepare: Use virStorageSource in def for SCSI hostdevs
On a Friday in 2020, Peter Krempa wrote: Modify the attach/detach data generators to actually use the virStorageSourceStructure embedded in the SCSI config data rather than creating an ad-hoc internal one. The modification will allow us to properly store the nodename used for the backend in the status XML rather than re-generating it all the time. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 71 +++-- 1 file changed, 47 insertions(+), 24 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 03:30:15PM +0200, Erik Skultety wrote: > On Mon, Sep 14, 2020 at 04:24:20PM +0300, Eli Cohen wrote: > > On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > > > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > > > > > $ ninja -C build > > > > > > ninja: Entering directory `build' > > > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom > > > > > > command > > > > > > FAILED: docs/manpages/virsh.html.in > > > > > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in > > > > > > -- > > > > > > /usr/local/bin/rst2html5 --stylesheet= --strict > > > > > > docs/manpages/virsh.rst > > > > > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > > > > installed a non-standard docutils build / version and that appears to > > > > > be breaking docs generation. > > > > > > > > > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > > > /usr/local/bin/. In anycase I did not put any package in /usr/local. In > > > > any case, I see different versions of rst2html5 in both /usr/bin and > > > > /usr/local/bin. > > > > > > It's not the build script that looks under /usr/local/bin, it's the PATH > > > variable which likely lists /usr/local/bin before /usr/bin. So, the only > > > explanation I have for you as for how rst2html5 appeared under > > > /usr/local/bin > > > is that you or someone else installed it with pip as root. > > > And the new docutils 0.16 looks like broke libvirt, so please stick with > > > 0.15. > > > > > I am on 0.15 > > > > I tried all the recommendations given but I have htting issues all the > > time. > > > > If anyone has it working over any Linux distribution, I am will to try > > that. I have a server ready for these experiments. I just need to have > > the latest libvirt running on my system. > > > > Well, if you look at our CI pipelines [1], you'll see that apart from Rawhide, > everything is green, so clearly the builds work. What other issues do you see? > Look at the Dockerfiles for the distros we have, install the packages you see > in the list and try re-running meson. That won't be enough - the bad rst5html5 install in /usr/local/ needs to be eliminated, either by deleting it, or removing it from $PATH. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 09/17] qemu: domain: Fill in (i)SCSI backend nodename if it is not present in status XML
On a Friday in 2020, Peter Krempa wrote: For upgrade reasons so that we can modify the used nodename we must generate the old version for all status XMLs which don't have it stored explicitly. The change will be required as using the user-provided alias may result in too-long nodenames which will be rejected by qemu. Add code which fills in the appropriate old value and add test cases to validate that it's added and also that existing nodenames are not overwritten. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 55 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 12 + 4 files changed, 69 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 04:24:20PM +0300, Eli Cohen wrote: > On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > > > $ ninja -C build > > > > > ninja: Entering directory `build' > > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom > > > > > command > > > > > FAILED: docs/manpages/virsh.html.in > > > > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > > > > > /usr/local/bin/rst2html5 --stylesheet= --strict > > > > > docs/manpages/virsh.rst > > > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > > > installed a non-standard docutils build / version and that appears to > > > > be breaking docs generation. > > > > > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > > /usr/local/bin/. In anycase I did not put any package in /usr/local. In > > > any case, I see different versions of rst2html5 in both /usr/bin and > > > /usr/local/bin. > > > > It's not the build script that looks under /usr/local/bin, it's the PATH > > variable which likely lists /usr/local/bin before /usr/bin. So, the only > > explanation I have for you as for how rst2html5 appeared under > > /usr/local/bin > > is that you or someone else installed it with pip as root. > > And the new docutils 0.16 looks like broke libvirt, so please stick with > > 0.15. > > > I am on 0.15 > > I tried all the recommendations given but I have htting issues all the > time. > > If anyone has it working over any Linux distribution, I am will to try > that. I have a server ready for these experiments. I just need to have > the latest libvirt running on my system. > Well, if you look at our CI pipelines [1], you'll see that apart from Rawhide, everything is green, so clearly the builds work. What other issues do you see? Look at the Dockerfiles for the distros we have, install the packages you see in the list and try re-running meson. Erik [1] https://gitlab.com/libvirt/libvirt/-/pipelines/189610405
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > $ ninja -C build > > > > ninja: Entering directory `build' > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom command > > > > FAILED: docs/manpages/virsh.html.in > > > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > > > > /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > > installed a non-standard docutils build / version and that appears to > > > be breaking docs generation. > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > /usr/local/bin/. In anycase I did not put any package in /usr/local. In > > any case, I see different versions of rst2html5 in both /usr/bin and > > /usr/local/bin. > > It's not the build script that looks under /usr/local/bin, it's the PATH > variable which likely lists /usr/local/bin before /usr/bin. So, the only > explanation I have for you as for how rst2html5 appeared under /usr/local/bin > is that you or someone else installed it with pip as root. > And the new docutils 0.16 looks like broke libvirt, so please stick with 0.15. > I am on 0.15 I tried all the recommendations given but I have htting issues all the time. If anyone has it working over any Linux distribution, I am will to try that. I have a server ready for these experiments. I just need to have the latest libvirt running on my system.
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 02:49:20PM +0200, Erik Skultety wrote: > On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > > > $ ninja -C build > > > > ninja: Entering directory `build' > > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom command > > > > FAILED: docs/manpages/virsh.html.in > > > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > > > > /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst > > > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > > installed a non-standard docutils build / version and that appears to > > > be breaking docs generation. > > > > > > > > > > Looks to me like the build scripts look for rst2html5 in > > /usr/local/bin/. In anycase I did not put any package in /usr/local. In > > any case, I see different versions of rst2html5 in both /usr/bin and > > /usr/local/bin. > > It's not the build script that looks under /usr/local/bin, it's the PATH > variable which likely lists /usr/local/bin before /usr/bin. So, the only > explanation I have for you as for how rst2html5 appeared under /usr/local/bin > is that you or someone else installed it with pip as root. > And the new docutils 0.16 looks like broke libvirt, so please stick with 0.15. It isn't docutils version that matters actually. It turns out there are 2 completely separate impls of rst2html5. One that is bundled with docutils on pip, which is the one libvirt works with. The other is in a rst2html5 package on pip. Basically need to remove the standalone rst2html5 pacakge that was installed from pip, or remove /usr/local/bin from $PATH so that it finds the from one docutils in /usr/bin Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 09/17] qemu: domain: Fill in (i)SCSI backend nodename if it is not present in status XML
On a Friday in 2020, Peter Krempa wrote: For upgrade reasons so that we can modify the used nodename we must generate the old version for all status XMLs which don't have it stored explicitly. The change will be required as using the user-provided alias may result in too-long nodenames which will be rejected by qemu. Add code which fills in the appropriate old value and add test cases to validate that it's added and also that existing nodenames are not overwritten. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 55 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 12 + 4 files changed, 69 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 08/17] tests: qemustatusxml2xmldata: Add local SCSI hostdev to 'upgrade' case
On a Friday in 2020, Peter Krempa wrote: Add a local SCSI host device to validate upcoming generated data. Signed-off-by: Peter Krempa --- tests/qemustatusxml2xmldata/upgrade-in.xml | 8 tests/qemustatusxml2xmldata/upgrade-out.xml | 8 2 files changed, 16 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 02:44:16PM +0300, Eli Cohen wrote: > On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > > > $ ninja -C build > > > ninja: Entering directory `build' > > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom command > > > FAILED: docs/manpages/virsh.html.in > > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > > > /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst > > > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > > installed a non-standard docutils build / version and that appears to > > be breaking docs generation. > > > > > > Looks to me like the build scripts look for rst2html5 in > /usr/local/bin/. In anycase I did not put any package in /usr/local. In > any case, I see different versions of rst2html5 in both /usr/bin and > /usr/local/bin. It's not the build script that looks under /usr/local/bin, it's the PATH variable which likely lists /usr/local/bin before /usr/bin. So, the only explanation I have for you as for how rst2html5 appeared under /usr/local/bin is that you or someone else installed it with pip as root. And the new docutils 0.16 looks like broke libvirt, so please stick with 0.15. Erik
Re: [libvirt PATCH 3/3] qemuProcessReconnect: clear 'oldjob'
On a Monday in 2020, Ján Tomko wrote:
After we started copying the privateData pointer in
qemuDomainObjRestoreJob, we should also free them
once we're done with them.
Register the clear function and use g_auto.
Also add a check for job->cb to qemuDomainObjClearJob,
to prevent freeing an uninitialized job.
https://bugzilla.redhat.com/show_bug.cgi?id=1878450
Signed-off-by: Ján Tomko
Fixes: aca37c3fb2e8d733c2788ca4b796c153ea7ce391
---
src/qemu/qemu_domainjob.c | 3 +++
src/qemu/qemu_domainjob.h | 1 +
src/qemu/qemu_process.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index e5910a11a1..3c2c6b9179 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -248,6 +248,9 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
void
qemuDomainObjClearJob(qemuDomainJobObjPtr job)
{
+if (!job->cb)
+return;
+
qemuDomainObjResetJob(job);
qemuDomainObjResetAsyncJob(job);
g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h
index eedd84c503..79f0127252 100644
--- a/src/qemu/qemu_domainjob.h
+++ b/src/qemu/qemu_domainjob.h
@@ -275,6 +275,7 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
bool qemuDomainTrackJob(qemuDomainJob job);
void qemuDomainObjClearJob(qemuDomainJobObjPtr job);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuDomainJobObj, qemuDomainObjClearJob);
int
qemuDomainObjInitJob(qemuDomainJobObjPtr job,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1af35b933..a345edd7fb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,7 +8081,7 @@ qemuProcessReconnect(void *opaque)
virQEMUDriverPtr driver = data->driver;
virDomainObjPtr obj = data->obj;
qemuDomainObjPrivatePtr priv;
-qemuDomainJobObj oldjob;
+g_auto(qemuDomainJobObj) oldjob = { 0 };
This fails on older compilers:
../src/qemu/qemu_process.c: In function ‘qemuProcessReconnect’:
../src/qemu/qemu_process.c:8084:5: error: missing braces around initializer
[-Werror=missing-braces]
g_auto(qemuDomainJobObj) oldjob = { 0 };
^
../src/qemu/qemu_process.c:8084:5: error: (near initialization for
‘oldjob.cond’) [-Werror=missing-braces]
../src/qemu/qemu_process.c:8084:5: error: missing initializer for field
‘active’ of ‘qemuDomainJobObj’ [-Werror=missing-field-initializers]
In file included from ../src/qemu/qemu_domain.h:34:0,
from ../src/qemu/qemu_process.h:25,
from ../src/qemu/qemu_process.c:41:
../src/qemu/qemu_domainjob.h:184:19: note: ‘active’ declared here
qemuDomainJob active; /* Currently running job */
^
cc1: all warnings being treated as errors
conisder this squashed in:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a345edd7fb..073b2d96e0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,7 +8081,9 @@ qemuProcessReconnect(void *opaque)
virQEMUDriverPtr driver = data->driver;
virDomainObjPtr obj = data->obj;
qemuDomainObjPrivatePtr priv;
-g_auto(qemuDomainJobObj) oldjob = { 0 };
+g_auto(qemuDomainJobObj) oldjob = {
+ .cb = NULL,
+};
int state;
int reason;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
signature.asc
Description: PGP signature
Re: [PATCH 5/5] node_device: mdev vfio-ccw support
On 9/14/20 7:34 AM, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 05:34:24PM +0200, Boris Fiuczynski wrote:
On 9/8/20 6:01 PM, Erik Skultety wrote:
@@ -664,11 +695,11 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
{
virCommandPtr cmd;
g_autofree char *json = NULL;
-g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
+g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
-if (!parent_pci) {
+if (!parent_addr) {
virReportError(VIR_ERR_NO_NODE_DEVICE,
- _("unable to find PCI address for parent device '%s'"),
def->parent);
+ _("unable to find address for parent device '%s'"),
def->parent);
I'm wondering whether "unable to find parent device '%s'" would not suffice,
since we're not specifying what type of address we were not able to find - I'm
not even sure the address information is important at all.
Erik
Erik,
how about
_("unable to find parent device '%s' by its address"), def->parent);
just to indicate the search criteria but I could also agree to a simple
_("unable to find parent device '%s'"), def->parent);
I'd still go with the latter.
Erik
OK, changed.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 4/5] node_device: detect DASD devices
On 9/14/20 7:17 AM, Erik Skultety wrote:
...
}
+/* dasd disk */
+if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+def->caps->data.storage.drive_type = g_strdup("dasd");
+VIR_DEBUG("Found storage type '%s' for device "
+ "with sysfs path '%s'",
+ def->caps->data.storage.drive_type,
+ def->sysfs_path);
I understand why we would need it for /dev/vdX, but can udev not know the
drive_type from kernel? IOW Do we really need ^this hunk?
For DASDs there are currently no identifies in udev besides ID_PATH.
ID_TYPE=disk does not exist. That's why the DASDs fall through the
udevProcessStorage detection logic. Without this hunk the dasd devices are
being detected as storage devices but than end up as "Unsupported storage
type".
The short answer is yes. :-)
Okay, can you put a concise version of ^this in a commentary then?
Erik
Done, I am going to send a v2 later.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH 2/5] node_device: detect CSS devices
On 9/14/20 7:41 AM, Erik Skultety wrote:
On Fri, Sep 11, 2020 at 06:11:49PM +0200, Boris Fiuczynski wrote:
On 9/9/20 9:03 AM, Cornelia Huck wrote:
[snip]
+static int
+udevProcessCSS(struct udev_device *device,
+ virNodeDeviceDefPtr def)
+{
+/* do not process EADM and CHSC devices to keep the list sane */
+if (STREQ(def->driver, "eadm_subchannel") ||
+STREQ(def->driver, "chsc_subchannel"))
[2] Also mentions message subchannel, although apparently there are no Linux
kernel drivers for that yet, IIUC that one would have to be added here as well
as some point, is that correct?
We have never seen those, but we would want to add them here if they
have no relevance for the user.
I think you want to filter message subchannels as well, my assumption
is that libvirt will only care about I/O subchannels.
In
https://www.kernel.org/doc/html/latest/driver-api/s390-drivers.html
it is stated that for
Message subchannels. No Linux driver currently exists.
Therefore I do not know how I could currently filter message subchannels
out. Due you want me to reverse the logic and just filter out everything but
"io_subchannel" instead (which I would regard as fencing the unknown)?
Well, I don't see a problem filtering out everything but the single thing you
actually care about, rather than enumerating the things you don't care about
and won't care about in the future, so yeah, I'd go with that.
Erik
OK, I will do that.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
[libvirt PATCH 2/3] qemu: qemuDomainObjClearJob: use g_clear_pointer
The function used g_clear_pointer for all but one pointer.
Signed-off-by: Ján Tomko
---
src/qemu/qemu_domainjob.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index 9f76f1ef04..e5910a11a1 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -250,7 +250,7 @@ qemuDomainObjClearJob(qemuDomainJobObjPtr job)
{
qemuDomainObjResetJob(job);
qemuDomainObjResetAsyncJob(job);
-job->cb->freeJobPrivate(job->privateData);
+g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
g_clear_pointer(&job->current, qemuDomainJobInfoFree);
g_clear_pointer(&job->completed, qemuDomainJobInfoFree);
virCondDestroy(&job->cond);
--
2.26.2
[libvirt PATCH 1/3] qemu: rename qemuDomainObjFreeJob -> qemuDomainObjClearJob
This function does not free the job.
Signed-off-by: Ján Tomko
---
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_domainjob.c | 2 +-
src/qemu/qemu_domainjob.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 785cee6f18..03917cf000 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1859,7 +1859,7 @@ qemuDomainObjPrivateFree(void *data)
qemuDomainObjPrivateDataClear(priv);
virObjectUnref(priv->monConfig);
-qemuDomainObjFreeJob(&priv->job);
+qemuDomainObjClearJob(&priv->job);
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index eeaa089959..9f76f1ef04 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -246,7 +246,7 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
}
void
-qemuDomainObjFreeJob(qemuDomainJobObjPtr job)
+qemuDomainObjClearJob(qemuDomainJobObjPtr job)
{
qemuDomainObjResetJob(job);
qemuDomainObjResetAsyncJob(job);
diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h
index fcbe3fe112..eedd84c503 100644
--- a/src/qemu/qemu_domainjob.h
+++ b/src/qemu/qemu_domainjob.h
@@ -274,7 +274,7 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
bool qemuDomainTrackJob(qemuDomainJob job);
-void qemuDomainObjFreeJob(qemuDomainJobObjPtr job);
+void qemuDomainObjClearJob(qemuDomainJobObjPtr job);
int
qemuDomainObjInitJob(qemuDomainJobObjPtr job,
--
2.26.2
[libvirt PATCH 0/3] fix a memory leak on qemuProcessReconnect
The first two patches have no functional impact on current code. Ján Tomko (3): qemu: rename qemuDomainObjFreeJob -> qemuDomainObjClearJob qemu: qemuDomainObjClearJob: use g_clear_pointer qemuProcessReconnect: clear 'oldjob' src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_domainjob.c | 7 +-- src/qemu/qemu_domainjob.h | 3 ++- src/qemu/qemu_process.c | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) -- 2.26.2
[libvirt PATCH 3/3] qemuProcessReconnect: clear 'oldjob'
After we started copying the privateData pointer in
qemuDomainObjRestoreJob, we should also free them
once we're done with them.
Register the clear function and use g_auto.
Also add a check for job->cb to qemuDomainObjClearJob,
to prevent freeing an uninitialized job.
https://bugzilla.redhat.com/show_bug.cgi?id=1878450
Signed-off-by: Ján Tomko
Fixes: aca37c3fb2e8d733c2788ca4b796c153ea7ce391
---
src/qemu/qemu_domainjob.c | 3 +++
src/qemu/qemu_domainjob.h | 1 +
src/qemu/qemu_process.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
index e5910a11a1..3c2c6b9179 100644
--- a/src/qemu/qemu_domainjob.c
+++ b/src/qemu/qemu_domainjob.c
@@ -248,6 +248,9 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
void
qemuDomainObjClearJob(qemuDomainJobObjPtr job)
{
+if (!job->cb)
+return;
+
qemuDomainObjResetJob(job);
qemuDomainObjResetAsyncJob(job);
g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h
index eedd84c503..79f0127252 100644
--- a/src/qemu/qemu_domainjob.h
+++ b/src/qemu/qemu_domainjob.h
@@ -275,6 +275,7 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
bool qemuDomainTrackJob(qemuDomainJob job);
void qemuDomainObjClearJob(qemuDomainJobObjPtr job);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuDomainJobObj, qemuDomainObjClearJob);
int
qemuDomainObjInitJob(qemuDomainJobObjPtr job,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1af35b933..a345edd7fb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,7 +8081,7 @@ qemuProcessReconnect(void *opaque)
virQEMUDriverPtr driver = data->driver;
virDomainObjPtr obj = data->obj;
qemuDomainObjPrivatePtr priv;
-qemuDomainJobObj oldjob;
+g_auto(qemuDomainJobObj) oldjob = { 0 };
int state;
int reason;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
--
2.26.2
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 12:14:40PM +0100, Daniel P. Berrangé wrote: > > > > $ ninja -C build > > ninja: Entering directory `build' > > [1124/1210] Generating virsh.html.in with a meson_exe.py custom command > > FAILED: docs/manpages/virsh.html.in > > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > > /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst > > We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've > installed a non-standard docutils build / version and that appears to > be breaking docs generation. > > Looks to me like the build scripts look for rst2html5 in /usr/local/bin/. In anycase I did not put any package in /usr/local. In any case, I see different versions of rst2html5 in both /usr/bin and /usr/local/bin.
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 02:08:55PM +0300, Eli Cohen wrote: > On Mon, Sep 14, 2020 at 11:38:06AM +0100, Daniel P. Berrangé wrote: > > Thanks for your reply, it is very helpful. > > > On Mon, Sep 14, 2020 at 01:11:01PM +0300, Eli Cohen wrote: > > > Hello all, > > > > > > I hope this is being posted in the right mailing list. > > > > > > I am trying to build libvirt v6.7.0 on RHEL8.3 x86_64 server in order to > > > later apply patches for VDPA support posted recently by Jonathon Jongsma > > > . > > > > > > I am following the instrutions in https://libvirt.org/contribute.html > > Link above is wrong, I really meant https://libvirt.org/compiling.html > > > > and running from the root directory: > > > > > > meson build --prefix=/usr > > > > > > I fail on the following error: > > > meson.build:921:2: ERROR: Program 'rpcgen portable-rpcgen' not found > > > > > > I can't find any reference how to overcome this obstacle. > > > > The rpcgen program is provided by the rpcgen RPM. eg > > > > $ dnf install rpcgen > > > > > Or more generally you can get almost everything libvirt needs by > > doing > > > > $ dnf builddep libvirt > > This does not work in my RHEL 8.3 machine but it does work on FC32. > After this I can successfuly run meson build --prefix=/usr > > Now after running ninja -C build > > I am getting some errors. Here's just the first two. > > $ ninja -C build > ninja: Entering directory `build' > [1124/1210] Generating virsh.html.in with a meson_exe.py custom command > FAILED: docs/manpages/virsh.html.in > /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- > /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst We'd expect to see that in /usr/bin/rst2html5 - it looks lke you've installed a non-standard docutils build / version and that appears to be breaking docs generation. > docs/manpages/virsh.rst:41: (ERROR/3) Error in "code-block" directive: > 1 argument(s) required, 0 supplied. > > .. code-block:: > >virsh [OPTION]... [ARG]... > > > Exiting due to level-3 (ERROR) system message. > [1140/1210] Generating virt-admin.html.in with a meson_exe.py custom > command > FAILED: docs/manpages/virt-admin.html.in > /usr/bin/meson --internal exe --capture docs/manpages/virt-admin.html.in > -- /usr/local/bin/rst2html5 --stylesheet= --strict > docs/manpages/virt-admin.rst > docs/manpages/virt-admin.rst:31: (ERROR/3) Error in "code-block" > directive: > 1 argument(s) required, 0 supplied. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 11:38:06AM +0100, Daniel P. Berrangé wrote: Thanks for your reply, it is very helpful. > On Mon, Sep 14, 2020 at 01:11:01PM +0300, Eli Cohen wrote: > > Hello all, > > > > I hope this is being posted in the right mailing list. > > > > I am trying to build libvirt v6.7.0 on RHEL8.3 x86_64 server in order to > > later apply patches for VDPA support posted recently by Jonathon Jongsma > > . > > > > I am following the instrutions in https://libvirt.org/contribute.html Link above is wrong, I really meant https://libvirt.org/compiling.html > > and running from the root directory: > > > > meson build --prefix=/usr > > > > I fail on the following error: > > meson.build:921:2: ERROR: Program 'rpcgen portable-rpcgen' not found > > > > I can't find any reference how to overcome this obstacle. > > The rpcgen program is provided by the rpcgen RPM. eg > > $ dnf install rpcgen > > Or more generally you can get almost everything libvirt needs by > doing > > $ dnf builddep libvirt This does not work in my RHEL 8.3 machine but it does work on FC32. After this I can successfuly run meson build --prefix=/usr Now after running ninja -C build I am getting some errors. Here's just the first two. $ ninja -C build ninja: Entering directory `build' [1124/1210] Generating virsh.html.in with a meson_exe.py custom command FAILED: docs/manpages/virsh.html.in /usr/bin/meson --internal exe --capture docs/manpages/virsh.html.in -- /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virsh.rst docs/manpages/virsh.rst:41: (ERROR/3) Error in "code-block" directive: 1 argument(s) required, 0 supplied. .. code-block:: virsh [OPTION]... [ARG]... Exiting due to level-3 (ERROR) system message. [1140/1210] Generating virt-admin.html.in with a meson_exe.py custom command FAILED: docs/manpages/virt-admin.html.in /usr/bin/meson --internal exe --capture docs/manpages/virt-admin.html.in -- /usr/local/bin/rst2html5 --stylesheet= --strict docs/manpages/virt-admin.rst docs/manpages/virt-admin.rst:31: (ERROR/3) Error in "code-block" directive: 1 argument(s) required, 0 supplied. > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| >
[libvirt PATCH] util: virNetDevTapCreate: initialize fd to -1
Signed-off-by: Ján Tomko Fixes: 95089f481e003d971fe0a082018216c58c1b80e5 --- Pushed as trivial. src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index ab5959c646..cbce5c71b7 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -322,7 +322,7 @@ int virNetDevTapCreate(char **ifname, size_t i = 0; struct ifreq ifr; int ret = -1; -int fd = 0; +int fd = -1; virMutexLock(&virNetDevTapCreateMutex); -- 2.26.2
Re: Issues with building libvirt v6.7.0 on RHEL8.3
On Mon, Sep 14, 2020 at 01:11:01PM +0300, Eli Cohen wrote: > Hello all, > > I hope this is being posted in the right mailing list. > > I am trying to build libvirt v6.7.0 on RHEL8.3 x86_64 server in order to > later apply patches for VDPA support posted recently by Jonathon Jongsma > . > > I am following the instrutions in https://libvirt.org/contribute.html > and running from the root directory: > > meson build --prefix=/usr > > I fail on the following error: > meson.build:921:2: ERROR: Program 'rpcgen portable-rpcgen' not found > > I can't find any reference how to overcome this obstacle. The rpcgen program is provided by the rpcgen RPM. eg $ dnf install rpcgen Or more generally you can get almost everything libvirt needs by doing $ dnf builddep libvirt Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Issues with building libvirt v6.7.0 on RHEL8.3
Hello all, I hope this is being posted in the right mailing list. I am trying to build libvirt v6.7.0 on RHEL8.3 x86_64 server in order to later apply patches for VDPA support posted recently by Jonathon Jongsma . I am following the instrutions in https://libvirt.org/contribute.html and running from the root directory: meson build --prefix=/usr I fail on the following error: meson.build:921:2: ERROR: Program 'rpcgen portable-rpcgen' not found I can't find any reference how to overcome this obstacle. Thanks for any help, Eli
Re: [PATCH v2] cphp: remove deprecated cpu-add command(s)
On Mon, 14 Sep 2020 10:07:36 +0200 Michal Privoznik wrote: > On 9/14/20 9:46 AM, Igor Mammedov wrote: > > theses were deprecated since 4.0, remove both HMP and QMP variants. > > > > Users should use device_add command instead. To get list of > > possible CPUs and options, use 'info hotpluggable-cpus' HMP > > or query-hotpluggable-cpus QMP command. > > > > Signed-off-by: Igor Mammedov > > Reviewed-by: Thomas Huth > > Acked-by: Dr. David Alan Gilbert > > --- > > include/hw/boards.h | 1 - > > include/hw/i386/pc.h| 1 - > > include/monitor/hmp.h | 1 - > > docs/system/deprecated.rst | 25 + > > hmp-commands.hx | 15 -- > > hw/core/machine-hmp-cmds.c | 12 - > > hw/core/machine-qmp-cmds.c | 12 - > > hw/i386/pc.c| 27 -- > > hw/i386/pc_piix.c | 1 - > > hw/s390x/s390-virtio-ccw.c | 12 - > > qapi/machine.json | 24 - > > tests/qtest/cpu-plug-test.c | 100 > > tests/qtest/test-hmp.c | 1 - > > 13 files changed, 21 insertions(+), 211 deletions(-) > > Thanks to Peter Libvirt uses device_add instead cpu_add whenever > possible. Hence this is okay from Libvirt's POV. we shoul make libvirt switch from -numa node,cpus= to -numa cpu= to get rid of the 'last' interface that uses cpu-index as input. To help libvirt to migrate existing configs from older syntax to the newer one, we can introduce field x-cpu-index to query-hotplugable-cpus output (with a goal to deprecate it in few years). Would it work for you? > > Reviewed-by: Michal Privoznik Thanks! > > Michal >
[libvirt PATCH 2/2] esx: implement domainInterfaceAddresses
Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.
Signed-off-by: Pino Toscano
---
docs/drvesx.html.in | 4 ++
src/esx/esx_driver.c | 166 +++
2 files changed, 170 insertions(+)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 38de640c2a..3acb7e5c51 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -799,6 +799,10 @@ Enter administrator password for example-vcenter.com:
virDomainGetHostname
+
+virDomainInterfaceAddresses (only for the
+VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+
virDomainReboot
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index bddc588977..0f63b5db4d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,171 @@ esxDomainGetHostname(virDomainPtr domain,
}
+static int
+esxParseIPAddress(const char *ipAddress, int prefixLength,
+ virDomainIPAddress *addr)
+{
+virSocketAddr tmp_addr;
+virIPAddrType addr_type;
+
+if (virSocketAddrParseAny(&tmp_addr, ipAddress, AF_UNSPEC, false) <= 0)
+return 0;
+
+switch (VIR_SOCKET_ADDR_FAMILY(&tmp_addr)) {
+case AF_INET:
+addr_type = VIR_IP_ADDR_TYPE_IPV4;
+break;
+case AF_INET6:
+addr_type = VIR_IP_ADDR_TYPE_IPV6;
+break;
+default:
+return 0;
+}
+
+addr->type = addr_type;
+addr->addr = g_strdup(ipAddress);
+addr->prefix = prefixLength;
+
+return 1;
+}
+
+
+static int
+esxDomainInterfaceAddresses(virDomainPtr domain,
+virDomainInterfacePtr **ifaces,
+unsigned int source,
+unsigned int flags)
+{
+int result = -1;
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+esxVI_DynamicProperty *dynamicProperty;
+esxVI_GuestNicInfo *guestNicInfoList = NULL;
+esxVI_GuestNicInfo *guestNicInfo = NULL;
+virDomainInterfacePtr *ifaces_ret = NULL;
+size_t ifaces_count = 0;
+size_t i;
+int ret;
+
+virCheckFlags(0, -1);
+if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Unknown IP address data source %d"),
+ source);
+return -1;
+}
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+if (esxVI_String_AppendValueListToList(&propertyNameList,
+ "runtime.powerState\0"
+ "guest.net") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, &virtualMachine,
+ esxVI_Occurrence_RequiredItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not powered on"));
+goto cleanup;
+}
+
+for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "guest.net")) {
+if (esxVI_GuestNicInfo_CastListFromAnyType
+ (dynamicProperty->val, &guestNicInfoList) < 0) {
+goto cleanup;
+}
+}
+}
+
+if (!guestNicInfoList)
+goto cleanup;
+
+for (guestNicInfo = guestNicInfoList; guestNicInfo;
+ guestNicInfo = guestNicInfo->_next) {
+virDomainInterfacePtr iface = NULL;
+size_t addrs_count = 0;
+
+if (guestNicInfo->connected != esxVI_Boolean_True ||
+!guestNicInfo->network) {
+continue;
+}
+
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface->naddrs = 0;
+iface->name = g_strdup(guestNicInfo->network);
+iface->hwaddr = g_strdup(guestNicInfo->macAddress);
+
+if (guestNicInfo->ipConfig) {
+esxVI_NetIpConfigInfoIpAddress *ipAddress;
+for (ipAddress = guestNicInfo->ipConfig->ipAddress; ipAddress;
+ ipAddress = ipAddress->_next) {
+virDomainIPAddress ip_addr;
+
+ret = esxParseIPAddress(ipAddress->ipAddress,
+ipAddress->prefixLength->value,
&ip_addr);
+if (ret < 0)
+
[libvirt PATCH 1/2] esx: generator: add GuestNicInfo object
Add the definition of the GuestNicInfo object, with all the required
objects for it.
Signed-off-by: Pino Toscano
---
scripts/esx_vi_generator.py| 1 +
src/esx/esx_vi_generator.input | 54 ++
2 files changed, 55 insertions(+)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index e0782e35f3..7929e1e682 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1292,6 +1292,7 @@ additional_object_features = {
"DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST |
Object.FEATURE__ANY_TYPE),
"DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST,
+"GuestNicInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
"HostConfigManager": Object.FEATURE__ANY_TYPE,
"HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
"HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST |
diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input
index 22c114e0aa..bd6ac72a18 100644
--- a/src/esx/esx_vi_generator.input
+++ b/src/esx/esx_vi_generator.input
@@ -277,6 +277,18 @@ object FolderFileQuery extends FileQuery
end
+object GuestNicInfo
+Boolean connected r
+Int deviceConfigId r
+NetDnsConfigInfo dnsConfig o
+String ipAddress ol
+NetIpConfigInfo ipConfig o
+String macAddress o
+NetBIOSConfigInfonetBIOSConfig o
+String networko
+end
+
+
object HostAutoStartManagerConfig
AutoStartDefaultsdefaults o
AutoStartPowerInfo powerInfo ol
@@ -770,6 +782,48 @@ object NasDatastoreInfo extends DatastoreInfo
end
+object NetBIOSConfigInfo
+String mode r
+end
+
+
+object NetDhcpConfigInfo
+NetDhcpConfigInfoDhcpOptions ipv4 o
+NetDhcpConfigInfoDhcpOptions ipv6 o
+end
+
+
+object NetDhcpConfigInfoDhcpOptions
+KeyAnyValue config ol
+Boolean enable r
+end
+
+
+object NetDnsConfigInfo
+Boolean dhcp r
+String domainName r
+String hostName r
+String ipAddress ol
+String searchDomain ol
+end
+
+
+object NetIpConfigInfo
+Boolean autoConfigurationEnabled o
+NetDhcpConfigInfodhcp o
+NetIpConfigInfoIpAddress ipAddress ol
+end
+
+
+object NetIpConfigInfoIpAddress
+String ipAddress r
+DateTime lifetime o
+String origin o
+Int prefixLength r
+String state o
+end
+
+
object ObjectContent
ManagedObjectReference objr
DynamicProperty propSetol
--
2.26.2
[libvirt PATCH] esx: implement connectListAllNetworks
Implement the .connectListAllNetworks networks API in the esx network
driver.
Signed-off-by: Pino Toscano
---
src/esx/esx_network_driver.c | 64
1 file changed, 64 insertions(+)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 88843aae2f..5d9c1e3c2c 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+ virNetworkPtr **nets,
+ unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+ &hostVirtualSwitchList) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch);
+
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free(&hostVirtualSwitchList);
+
+return ret;
+}
+#undef MATCH
+
+
+
virNetworkDriver esxNetworkDriver = {
.connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
.connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +940,5 @@ virNetworkDriver esxNetworkDriver = {
.networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
.networkIsActive = esxNetworkIsActive, /* 0.10.0 */
.networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.8.0 */
};
--
2.26.2
[PATCH v5 0/8] Configurable policy for handling deprecated interfaces
New option -compat lets you configure what to do when deprecated interfaces get used. This is intended for testing users of the management interfaces. It is experimental. -compat deprecated-input= configures what to do when deprecated input is received. Available policies: * accept: Accept deprecated commands and arguments (default) * reject: Reject them * crash: Crash -compat deprecated-output= configures what to do when deprecated output is sent. Available output policies: * accept: Emit deprecated command results and events (default) * hide: Suppress them For now, -compat covers only deprecated syntactic aspects of QMP. We may want to extend it to cover semantic aspects, CLI, and experimental features. v5: * Old PATCH 01-26 merged in commit f57587c7d47. * Rebased, non-trivial conflicts in PATCH 1 due to Meson, and in PATCH 7 due to visitor changes * PATCH 1: Comments updated for 5.2 [Eric] * PATCH 2: Harmless missing initialization fixed [Eric] * PATCH 3+4: Harmless missing has_FOO = true fixed [Eric] * PATCH 6+7: Commit message tweaked v4: * PATCH 05+07: Temporary memory leak plugged [Marc-André] * PATCH 23: Rewritten [Marc-André] * PATCH 24: Comment typo [Marc-André] * PATCH 30: Memory leaks plugged v3: * Rebased, non-trivial conflicts in PATCH 01+26+27+34 due to RST conversion and code motion * PATCH 28-29: Old PATCH 28 split up to ease review * PATCH 30-31: New * PATCH 32-33: Old PATCH 29 split up to ease review Comparison to RFC (24 Oct 2019): * Cover arguments and results in addition to commands and events * Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command" dropped See also last item of Subject: Minutes of KVM Forum BoF on deprecating stuff Date: Fri, 26 Oct 2018 16:03:51 +0200 Message-ID: <87mur0ls8o@dusky.pond.sub.org> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html Cc: Lukáš Doktor Cc: libgues...@redhat.com Cc: libvir-list@redhat.com Cc: Daniel P. Berrange Cc: Peter Krempa Cc: Kevin Wolf Markus Armbruster (8): qemu-options: New -compat to set policy for deprecated interfaces qapi: Implement deprecated-output=hide for QMP command results qapi: Implement deprecated-output=hide for QMP events qapi: Implement deprecated-output=hide for QMP event data qapi: Implement deprecated-output=hide for QMP introspection qapi: Implement deprecated-input=reject for QMP commands qapi: Implement deprecated-input=reject for QMP command arguments qapi: New -compat deprecated-input=crash qapi/compat.json| 52 qapi/introspect.json| 2 +- qapi/qapi-schema.json | 1 + include/qapi/compat-policy.h| 20 + include/qapi/qmp/dispatch.h | 1 + include/qapi/qobject-input-visitor.h| 9 +++ include/qapi/qobject-output-visitor.h | 9 +++ include/qapi/visitor-impl.h | 6 ++ include/qapi/visitor.h | 18 + monitor/monitor-internal.h | 3 - monitor/misc.c | 2 - monitor/qmp-cmds-control.c | 100 +--- qapi/qapi-visit-core.c | 18 + qapi/qmp-dispatch.c | 17 qapi/qobject-input-visitor.c| 29 +++ qapi/qobject-output-visitor.c | 19 + softmmu/vl.c| 17 storage-daemon/qemu-storage-daemon.c| 2 - tests/test-qmp-cmds.c | 91 +++-- tests/test-qmp-event.c | 41 ++ qapi/meson.build| 1 + qapi/trace-events | 2 + qemu-options.hx | 22 ++ scripts/qapi/commands.py| 14 ++-- scripts/qapi/events.py | 22 +- scripts/qapi/visit.py | 15 tests/qapi-schema/qapi-schema-test.json | 20 +++-- tests/qapi-schema/qapi-schema-test.out | 20 ++--- 28 files changed, 522 insertions(+), 51 deletions(-) create mode 100644 qapi/compat.json create mode 100644 include/qapi/compat-policy.h -- 2.26.2
Re: [libvirt PATCH v2] esx: improve some of the virErrorNumber used
On Thu, Sep 10, 2020 at 03:26:03PM +0200, Pino Toscano wrote: A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent the number of the error, even in cases where there is one fitting more. Hence, replace some of them with better virErrorNumber values. Signed-off-by: Pino Toscano Reviewed-by: Martin Kletzander signature.asc Description: PGP signature
Re: [libvirt PATCH v2] esx: generator: fix free of elements in lists
On Thu, Sep 10, 2020 at 03:10:00PM +0200, Pino Toscano wrote:
When a list is freed, we iterate through all the items, invoking the
free function for each; the actual free function called for each element
is the function of the actual type of each element, and thus the @_next
pointer in the element struct has the same type as the element itself.
Currently, the free function gets the parent of the current element
type, and invoke its free function to continue freeing the list.
However, in case the hierarchy of the classes has more than 1 level
(i.e. Class <- SubClass <- SubSubClass), the invoked free function is
only the parent class' one, and not the actual base class of the
hierarchy.
To fix that, change the generator to get the base class of a class, and
invoking that instead. Also, avoid to set the @_next back, as it is not
needed.
Fixes commits 5cff36e39ae691fbd7c40597df1732eecf294150 and
f76c6dde2e33233566e886d96e76b5fe0c102d9a.
Signed-off-by: Pino Toscano
Reviewed-by: Martin Kletzander
---
Changes in v2:
- ancestor -> base class
scripts/esx_vi_generator.py | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 863c8af964..e0782e35f3 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -751,13 +751,13 @@ class Object(GenericObject):
source += "{\n"
if self.features & Object.FEATURE__LIST:
-if self.extends is not None:
+base_class = get_base_class(self)
+if base_class:
# avoid "dereferencing type-punned pointer will break
# strict-aliasing rules" warnings
-source += "esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \
- % (self.extends, self.extends)
-source += "esxVI_%s_Free(&next);\n" % self.extends
-source += "item->_next = (esxVI_%s *)next;\n\n" % self.name
+source += "esxVI_%s *baseNext = (esxVI_%s
*)item->_next;\n" \
+ % (base_class, base_class)
+source += "esxVI_%s_Free(&baseNext);\n\n" % base_class
else:
source += "esxVI_%s_Free(&item->_next);\n\n" % self.name
@@ -1250,6 +1250,21 @@ def is_known_type(type):
type in enums_by_name)
+def get_base_class(obj):
+if not obj.extends:
+return None
+base_class = None
+try:
+base_class = base_class_by_name[obj.extends]
+except KeyError:
+parent = objects_by_name[obj.extends]
+base_class = get_base_class(parent)
+if not base_class:
+base_class = parent.name
+base_class_by_name[name] = base_class
+return base_class
+
+
def open_file(filename):
return open(filename, "wt")
@@ -1341,6 +1356,7 @@ managed_objects_by_name = {}
enums_by_name = {}
methods_by_name = {}
block = None
+base_class_by_name = {}
# parse input file
--
2.26.2
signature.asc
Description: PGP signature
Re: [PATCH v2] cphp: remove deprecated cpu-add command(s)
On 9/14/20 9:46 AM, Igor Mammedov wrote: theses were deprecated since 4.0, remove both HMP and QMP variants. Users should use device_add command instead. To get list of possible CPUs and options, use 'info hotpluggable-cpus' HMP or query-hotpluggable-cpus QMP command. Signed-off-by: Igor Mammedov Reviewed-by: Thomas Huth Acked-by: Dr. David Alan Gilbert --- include/hw/boards.h | 1 - include/hw/i386/pc.h| 1 - include/monitor/hmp.h | 1 - docs/system/deprecated.rst | 25 + hmp-commands.hx | 15 -- hw/core/machine-hmp-cmds.c | 12 - hw/core/machine-qmp-cmds.c | 12 - hw/i386/pc.c| 27 -- hw/i386/pc_piix.c | 1 - hw/s390x/s390-virtio-ccw.c | 12 - qapi/machine.json | 24 - tests/qtest/cpu-plug-test.c | 100 tests/qtest/test-hmp.c | 1 - 13 files changed, 21 insertions(+), 211 deletions(-) Thanks to Peter Libvirt uses device_add instead cpu_add whenever possible. Hence this is okay from Libvirt's POV. Reviewed-by: Michal Privoznik Michal
[libvirt PATCH v2 5/7] tests: Use glib memory function in testConfRoundTrip
Signed-off-by: Tim Wiederhake
---
tests/virconftest.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c
index cac3718495..8269730662 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -32,39 +32,30 @@
static int testConfRoundTrip(const void *opaque)
{
const char *name = opaque;
-int ret = -1;
g_autoptr(virConf) conf = NULL;
int len = 1;
-char *buffer = NULL;
-char *srcfile = NULL;
-char *dstfile = NULL;
+g_autofree char *buffer = NULL;
+g_autofree char *srcfile = NULL;
+g_autofree char *dstfile = NULL;
srcfile = g_strdup_printf("%s/virconfdata/%s.conf", abs_srcdir, name);
dstfile = g_strdup_printf("%s/virconfdata/%s.out", abs_srcdir, name);
-if (VIR_ALLOC_N_QUIET(buffer, len) < 0) {
-fprintf(stderr, "out of memory\n");
-goto cleanup;
-}
+buffer = g_new0(char, len);
conf = virConfReadFile(srcfile, 0);
if (conf == NULL) {
fprintf(stderr, "Failed to process %s\n", srcfile);
-goto cleanup;
+return -1;
}
if (virConfWriteMem(buffer, &len, conf) < 0) {
fprintf(stderr, "Failed to serialize %s back\n", srcfile);
-goto cleanup;
+return -1;
}
if (virTestCompareToFile(buffer, dstfile) < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-VIR_FREE(srcfile);
-VIR_FREE(dstfile);
-VIR_FREE(buffer);
-return ret;
+return 0;
}
--
2.26.2
[libvirt PATCH v2 6/7] util: Use glib memory functions in virJSONValueGetArrayAsBitmap
Signed-off-by: Tim Wiederhake
---
src/util/virjson.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 6921eccb60..ba43d6b667 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1243,8 +1243,7 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val,
if (val->type != VIR_JSON_TYPE_ARRAY)
return -1;
-if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0)
-return -1;
+elems = g_new0(unsigned long long, val->data.array.nvalues);
/* first pass converts array members to numbers and finds the maximum */
for (i = 0; i < val->data.array.nvalues; i++) {
--
2.26.2
[libvirt PATCH v2 7/7] util: Remove VIR_ALLOC_N_QUIET
Signed-off-by: Tim Wiederhake Reviewed-by: Ján Tomko --- src/util/viralloc.h | 15 --- 1 file changed, 15 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index fa2bc8a2bc..a50cd5d632 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -90,21 +90,6 @@ void virDisposeString(char **strptr) */ #define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count)) -/** - * VIR_ALLOC_N_QUIET: - * @ptr: pointer to hold address of allocated memory - * @count: number of elements to allocate - * - * Allocate an array of 'count' elements, each sizeof(*ptr) - * bytes long and store the address of allocated memory in - * 'ptr'. Fill the newly allocated memory with zeros. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_ALLOC_N_QUIET(ptr, count) VIR_ALLOC_N(ptr, count) - /** * VIR_REALLOC_N: * @ptr: pointer to hold address of allocated memory -- 2.26.2
[libvirt PATCH v2 4/7] util: Remove VIR_ALLOC_QUIET
Signed-off-by: Tim Wiederhake Reviewed-by: Ján Tomko --- src/util/viralloc.h | 14 -- 1 file changed, 14 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 3e57d8a603..fa2bc8a2bc 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -75,20 +75,6 @@ void virDisposeString(char **strptr) */ #define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr))) -/** - * VIR_ALLOC_QUIET: - * @ptr: pointer to hold address of allocated memory - * - * Allocate sizeof(*ptr) bytes of memory and store - * the address of allocated memory in 'ptr'. Fill the - * newly allocated memory with zeros. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_ALLOC_QUIET(ptr) VIR_ALLOC(ptr) - /** * VIR_ALLOC_N: * @ptr: pointer to hold address of allocated memory -- 2.26.2
[libvirt PATCH v2 3/7] util: Use glib memory functions in virLastErrorObject
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index d89948f198..80a7cfe0ed 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -235,10 +235,9 @@ virLastErrorObject(void)
virErrorPtr err;
err = virThreadLocalGet(&virLastErr);
if (!err) {
-if (VIR_ALLOC_QUIET(err) < 0)
-return NULL;
+err = g_new0(virError, 1);
if (virThreadLocalSet(&virLastErr, err) < 0)
-VIR_FREE(err);
+g_clear_pointer(&err, g_free);
}
return err;
}
--
2.26.2
[libvirt PATCH v2 2/7] util: Use glib memory functions in virErrorCopyNew
Signed-off-by: Tim Wiederhake
---
src/util/virerror.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 6b057057a3..d89948f198 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -223,14 +223,8 @@ virCopyError(virErrorPtr from,
virErrorPtr
virErrorCopyNew(virErrorPtr err)
{
-virErrorPtr ret;
-
-if (VIR_ALLOC_QUIET(ret) < 0)
-return NULL;
-
-if (virCopyError(err, ret) < 0)
-VIR_FREE(ret);
-
+virErrorPtr ret = g_new0(virError, 1);
+virCopyError(err, ret);
return ret;
}
--
2.26.2
[libvirt PATCH v2 0/7] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.
V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00633.html Tim Wiederhake (7): tests: Fix false positive in testConfRoundTrip. util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET src/util/viralloc.h | 29 - src/util/virerror.c | 15 --- src/util/virjson.c | 3 +-- tests/virconftest.c | 28 +--- 4 files changed, 14 insertions(+), 61 deletions(-) -- 2.26.2
[libvirt PATCH v2 1/7] tests: Fix false positive in testConfRoundTrip.
testConfRoundTrip would return 0 (success) if virConfWriteMem returned 0
(nothing
written) and virTestCompareToFile failed.
Signed-off-by: Tim Wiederhake
---
tests/virconftest.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c
index ab29b5b712..cac3718495 100644
--- a/tests/virconftest.c
+++ b/tests/virconftest.c
@@ -51,8 +51,7 @@ static int testConfRoundTrip(const void *opaque)
fprintf(stderr, "Failed to process %s\n", srcfile);
goto cleanup;
}
-ret = virConfWriteMem(buffer, &len, conf);
-if (ret < 0) {
+if (virConfWriteMem(buffer, &len, conf) < 0) {
fprintf(stderr, "Failed to serialize %s back\n", srcfile);
goto cleanup;
}
--
2.26.2
[PATCH 2/2] qemu: Use .hostdevice attribute for usb-host
This originally started as bug 1595525 in which namespaces and
libusb used in QEMU were not playing nicely with each other. The
problem was that libusb built a cache of USB devices it saw
(which was a very limited set because of the namespace) and then
expected to receive udev events to keep the cache in sync. But
those udev events didn't come because on hotplug when we mknod()
devices in the namespace no udev event is generated. And what is
worse - libusb failed to open a device that wasn't in the cache.
Without going further into what the problem was, libusb added a
new API for opening USB devices that avoids using cache which
QEMU incorporated and exposes under "hostdevice" attribute.
What is even nicer is that QEMU uses qemu_open() for path
provided in the attribute and thus FD passing could be used.
Except qemu_open() expects so called FD sets instead of `getfd'
and these are not implemented in Libvirt, yet.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1877218
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bd98b0a97c..4f85848d32 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4460,16 +4460,21 @@ qemuBuildUSBHostdevDevStr(const virDomainDef *def,
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
-if (!dev->missing && !usbsrc->bus && !usbsrc->device) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("USB host device is missing bus/device information"));
-return NULL;
-}
-
virBufferAddLit(&buf, "usb-host");
if (!dev->missing) {
-virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
- usbsrc->bus, usbsrc->device);
+if (usbsrc->bus == 0 && usbsrc->device == 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("USB host device is missing bus/device
information"));
+return NULL;
+}
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HOST_HOSTDEVICE)) {
+virBufferAsprintf(&buf, ",hostdevice=/dev/bus/usb/%03d/%03d",
+ usbsrc->bus, usbsrc->device);
+} else {
+virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
+ usbsrc->bus, usbsrc->device);
+}
}
virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
if (dev->info->bootIndex)
--
2.26.2
[PATCH 0/2] qemu: Use .hostdevice attribute for usb-host
See 2/2 for explanation why we need this. Also, .hostdevice uses qemu_open() under the hood which enables Libvirt to pass FD instead of /dev/bus/usb/... path. However, qemu_open() does not support getfd style of passing FD (which is what we use), but so called FD sets which require some more work. I'm working on it as we speak but I figured, let's fix this bug first and post FD passing after that. For curious ones, here is the raw, unclean version with FD sets: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/qemu_hostdev_alt/ Michal Prívozník (2): qemu_capabilities: Add QEMU_CAPS_USB_HOST_HOSTDEVICE qemu: Use .hostdevice attribute for usb-host src/qemu/qemu_capabilities.c | 10 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 21 ++- .../caps_1.5.3.x86_64.replies | 126 -- .../caps_1.6.0.x86_64.replies | 126 -- .../caps_1.7.0.x86_64.replies | 126 -- .../caps_2.1.1.x86_64.replies | 126 -- .../caps_2.10.0.aarch64.replies | 134 +-- .../caps_2.10.0.ppc64.replies | 130 -- .../caps_2.10.0.s390x.replies | 134 +-- .../caps_2.10.0.x86_64.replies| 146 +--- .../caps_2.11.0.s390x.replies | 134 +-- .../caps_2.11.0.x86_64.replies| 146 +--- .../caps_2.12.0.aarch64.replies | 150 +--- .../caps_2.12.0.ppc64.replies | 142 --- .../caps_2.12.0.s390x.replies | 142 --- .../caps_2.12.0.x86_64.replies| 162 ++ .../caps_2.4.0.x86_64.replies | 126 -- .../caps_2.5.0.x86_64.replies | 130 -- .../caps_2.6.0.aarch64.replies| 134 +-- .../caps_2.6.0.ppc64.replies | 130 -- .../caps_2.6.0.x86_64.replies | 130 -- .../caps_2.7.0.s390x.replies | 130 -- .../caps_2.7.0.x86_64.replies | 130 -- .../caps_2.8.0.s390x.replies | 134 +-- .../caps_2.8.0.x86_64.replies | 130 -- .../caps_2.9.0.ppc64.replies | 130 -- .../caps_2.9.0.s390x.replies | 134 +-- .../caps_2.9.0.x86_64.replies | 146 +--- .../caps_3.0.0.ppc64.replies | 142 --- .../caps_3.0.0.riscv32.replies| 138 +-- .../caps_3.0.0.riscv64.replies| 138 +-- .../caps_3.0.0.s390x.replies | 142 --- .../caps_3.0.0.x86_64.replies | 162 ++ .../caps_3.1.0.ppc64.replies | 142 --- .../caps_3.1.0.x86_64.replies | 162 ++ .../caps_4.0.0.aarch64.replies| 150 +--- .../caps_4.0.0.ppc64.replies | 142 --- .../caps_4.0.0.riscv32.replies| 138 +-- .../caps_4.0.0.riscv64.replies| 138 +-- .../caps_4.0.0.s390x.replies | 142 --- .../caps_4.0.0.x86_64.replies | 162 ++ .../caps_4.1.0.x86_64.replies | 154 ++--- .../caps_4.2.0.aarch64.replies| 154 ++--- .../caps_4.2.0.ppc64.replies | 142 --- .../caps_4.2.0.s390x.replies | 142 --- .../caps_4.2.0.x86_64.replies | 154 ++--- .../caps_5.0.0.aarch64.replies| 154 ++--- .../caps_5.0.0.ppc64.replies | 142 --- .../caps_5.0.0.riscv64.replies| 138 +-- .../caps_5.0.0.x86_64.replies | 154 ++--- .../caps_5.1.0.x86_64.replies | 158 ++--- .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.replies | 158 ++--- .../caps_5.2.0.x86_64.xml | 1 + 55 files changed, 6110 insertions(+), 982 deletions(-) -- 2.26.2
[PATCH v2] cphp: remove deprecated cpu-add command(s)
theses were deprecated since 4.0, remove both HMP and QMP variants.
Users should use device_add command instead. To get list of
possible CPUs and options, use 'info hotpluggable-cpus' HMP
or query-hotpluggable-cpus QMP command.
Signed-off-by: Igor Mammedov
Reviewed-by: Thomas Huth
Acked-by: Dr. David Alan Gilbert
---
include/hw/boards.h | 1 -
include/hw/i386/pc.h| 1 -
include/monitor/hmp.h | 1 -
docs/system/deprecated.rst | 25 +
hmp-commands.hx | 15 --
hw/core/machine-hmp-cmds.c | 12 -
hw/core/machine-qmp-cmds.c | 12 -
hw/i386/pc.c| 27 --
hw/i386/pc_piix.c | 1 -
hw/s390x/s390-virtio-ccw.c | 12 -
qapi/machine.json | 24 -
tests/qtest/cpu-plug-test.c | 100
tests/qtest/test-hmp.c | 1 -
13 files changed, 21 insertions(+), 211 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 795910d01b..7abd5d889c 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -169,7 +169,6 @@ struct MachineClass {
void (*init)(MachineState *state);
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
-void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
int (*kvm_type)(MachineState *machine, const char *arg);
void (*smp_parse)(MachineState *ms, QemuOpts *opts);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 421a77acc2..79b7ab17bc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -135,7 +135,6 @@ extern int fd_bootchk;
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
void pc_smp_parse(MachineState *ms, QemuOpts *opts);
void pc_guest_info_init(PCMachineState *pcms);
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index c986cfd28b..642e9e91f9 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -89,7 +89,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_change(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_chardev_send_break(Monitor *mon, const QDict *qdict);
-void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index a158e765c3..c43c53f432 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -284,13 +284,6 @@ The ``query-cpus`` command is replaced by the
``query-cpus-fast`` command.
The ``arch`` output member of the ``query-cpus-fast`` command is
replaced by the ``target`` output member.
-``cpu-add`` (since 4.0)
-'''
-
-Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
-documentation of ``query-hotpluggable-cpus`` for additional
-details.
-
``query-events`` (since 4.0)
@@ -306,12 +299,6 @@ the 'wait' field, which is only applicable to sockets in
server mode
Human Monitor Protocol (HMP) commands
-
-``cpu-add`` (since 4.0)
-'''
-
-Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
-documentation of ``query-hotpluggable-cpus`` for additional details.
-
``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove``
(since 4.0.0)
''
@@ -521,6 +508,12 @@ QEMU Machine Protocol (QMP) commands
The "autoload" parameter has been ignored since 2.12.0. All bitmaps
are automatically loaded from qcow2 images.
+``cpu-add`` (removed in 5.2)
+
+
+Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
+documentation of ``query-hotpluggable-cpus`` for additional details.
+
Human Monitor Protocol (HMP) commands
-
@@ -530,6 +523,12 @@ The ``hub_id`` parameter of ``hostfwd_add`` /
``hostfwd_remove`` (removed in 5.0
The ``[hub_id name]`` parameter tuple of the 'hostfwd_add' and
'hostfwd_remove' HMP commands has been replaced by ``netdev_id``.
+``cpu-add`` (removed in 5.2)
+
+
+Use ``device_add`` for hotplugging vCPUs instead of ``cpu-add``. See
+documentation of ``query-hotpluggable-cpus`` for additional details.
+
Guest Emulator ISAs
---
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 60f395c276..d1e3e0e1c6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1761,21 +1761,6 @@ SRST
Executes a qemu-io command on the given block device.
ERST
-{
-.name = "cpu-add",
-.args_type = "id:i",
-.params = "id",
-.help = "add cpu (deprecat
Re: [PATCH] smp: drop support for deprecated (invalid topologies)
On Fri, 11 Sep 2020 11:04:47 -0400
"Michael S. Tsirkin" wrote:
> On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
> > it's was deprecated since 3.1
> >
> > Support for invalid topologies is removed, the user must ensure
> > that topologies described with -smp include all possible cpus,
> > i.e. (sockets * cores * threads) == maxcpus or QEMU will
> > exit with error.
> >
> > Signed-off-by: Igor Mammedov
>
> Acked-by:
>
> memory tree I guess?
It would be better for Paolo to take it since he has
queued numa deprecations, due to context confilict in
deprecated.rst.
Paolo,
can you queue this patch as well?
>
> > ---
> > docs/system/deprecated.rst | 26 +-
> > hw/core/machine.c | 16
> > hw/i386/pc.c | 16
> > 3 files changed, 21 insertions(+), 37 deletions(-)
> >
> > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> > index 122717cfee..d737728fab 100644
> > --- a/docs/system/deprecated.rst
> > +++ b/docs/system/deprecated.rst
> > @@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate
> > for character or host
> > devices and will only accept regular files (S_IFREG). The correct driver
> > for these file types is 'host_cdrom' or 'host_device' as appropriate.
> >
> > -``-smp`` (invalid topologies) (since 3.1)
> > -'
> > -
> > -CPU topology properties should describe whole machine topology including
> > -possible CPUs.
> > -
> > -However, historically it was possible to start QEMU with an incorrect
> > topology
> > -where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> > -which could lead to an incorrect topology enumeration by the guest.
> > -Support for invalid topologies will be removed, the user must ensure
> > -topologies described with -smp include all possible cpus, i.e.
> > -*sockets* * *cores* * *threads* = *maxcpus*.
> > -
> > ``-vnc acl`` (since 4.0.0)
> > ''
> >
> > @@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the
> > option but it will still
> > work with old machine types. User can check the QAPI schema to see if the
> > legacy
> > option is supported by looking at MachineInfo::numa-mem-supported property.
> >
> > +``-smp`` (invalid topologies) (removed 5.2)
> > +'''
> > +
> > +CPU topology properties should describe whole machine topology including
> > +possible CPUs.
> > +
> > +However, historically it was possible to start QEMU with an incorrect
> > topology
> > +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> > +which could lead to an incorrect topology enumeration by the guest.
> > +Support for invalid topologies is removed, the user must ensure
> > +topologies described with -smp include all possible cpus, i.e.
> > +*sockets* * *cores* * *threads* = *maxcpus*.
> > +
> > Block devices
> > -
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index ea26d61237..09aee4ea52 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts
> > *opts)
> > exit(1);
> > }
> >
> > -if (sockets * cores * threads > ms->smp.max_cpus) {
> > -error_report("cpu topology: "
> > - "sockets (%u) * cores (%u) * threads (%u) > "
> > - "maxcpus (%u)",
> > +if (sockets * cores * threads != ms->smp.max_cpus) {
> > +error_report("Invalid CPU topology: "
> > + "sockets (%u) * cores (%u) * threads (%u) "
> > + "!= maxcpus (%u)",
> > sockets, cores, threads,
> > ms->smp.max_cpus);
> > exit(1);
> > }
> >
> > -if (sockets * cores * threads != ms->smp.max_cpus) {
> > -warn_report("Invalid CPU topology deprecated: "
> > -"sockets (%u) * cores (%u) * threads (%u) "
> > -"!= maxcpus (%u)",
> > -sockets, cores, threads,
> > -ms->smp.max_cpus);
> > -}
> > -
> > ms->smp.cpus = cpus;
> > ms->smp.cores = cores;
> > ms->smp.threads = threads;
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index d071da787b..fbde6b04e6 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
> > exit(1);
> > }
> >
> > -if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> > -error_report("cpu topology: "
> > - "sockets (%u) * dies (%u) * cores (%u) * threads
> > (%u) > "
> > - "maxcpus (%u)",
> > +if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> > +error_report(
