July 2021 - Devel - libvirt List Archives

[libvirt PATCH] virThreadPoolExpand: Prevent expanding worker pool by zero
by Tim Wiederhake 12 Jul '21

12 Jul '21

`virThreadPoolNewFull` may call `virThreadPoolExpand` with `prioWorkers` = 0. This causes `virThreadPoolExpand` to call `VIR_EXPAND_N` on a null pointer and an increment of zero. The zero increment triggers `virReallocN` to not actually allocate any memory and leave the pointer NULL, which, eventually, causes `memset(NULL, 0, 0)` to be called in `virExpandN`. `memset` is declared `__attribute__ ((__nonnull__ 1))`, which triggers the following warning when libvirt is compiled with address sanitizing enabled: src/util/viralloc.c:82:5: runtime error: null pointer passed as argument 1, which is declared to never be null Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/util/virthreadpool.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 9ddd86a679..c9d2a17ff4 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -179,6 +179,9 @@ virThreadPoolExpand(virThreadPool *pool, size_t gain, bool priority) size_t i = 0; struct virThreadPoolWorkerData *data = NULL; + if (gain == 0) + return 0; + VIR_EXPAND_N(*workers, *curWorkers, gain); for (i = 0; i < gain; i++) { -- 2.31.1

3 4

[PATCH] qemu: Fix crash in virQEMUCapsProbeQMPTPM
by Peter Krempa 12 Jul '21

12 Jul '21

If the queried QMP command doesn't exist qemuMonitorGetTPMModels returns 0 but sets the string list to NULL which isn't accepted by g_strv_contains. Fixes: a5bc5f0ecf8 Reported-by: Olaf Hering <olaf(a)aepfle.de> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_capabilities.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d1cd8f11ac..5b359258a1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3168,6 +3168,9 @@ virQEMUCapsProbeQMPTPM(virQEMUCaps *qemuCaps, if (qemuMonitorGetTPMModels(mon, &models) < 0) return -1; + if (!models) + return 0; + for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMModelsToCaps); i++) { const char *needle = virDomainTPMModelTypeToString(virQEMUCapsTPMModelsToCaps[i].type); if (g_strv_contains((const char **)models, needle)) @@ -3177,10 +3180,12 @@ virQEMUCapsProbeQMPTPM(virQEMUCaps *qemuCaps, if (qemuMonitorGetTPMTypes(mon, &types) < 0) return -1; - for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) { - const char *needle = virDomainTPMBackendTypeToString(virQEMUCapsTPMTypesToCaps[i].type); - if (g_strv_contains((const char **)types, needle)) - virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps); + if (types) { + for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) { + const char *needle = virDomainTPMBackendTypeToString(virQEMUCapsTPMTypesToCaps[i].type); + if (g_strv_contains((const char **)types, needle)) + virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps); + } } return 0; -- 2.31.1

2 2

[libvirt PATCH 0/6] More XML parsing boiler code refactoring
by Tim Wiederhake 12 Jul '21

12 Jul '21

In the spirit of https://listman.redhat.com/archives/libvir-list/2021-May/msg00550.html. Tim Wiederhake (6): conf: Add AUTOPTR_CLEANUP_FUNC for virDomainSEVDef conf: virDomainSEVDef: Change type of "sectype" to virDomainLaunchSecurity virDomainSEVDefParseXML: Use virXMLPropEnum virDomainSEVDefParseXML: Use automatic memory management virDomainSEVDefParseXML: Remove superfluous `goto`s virDomainSEVDefParseXML: Remove superfluous variable initialization src/conf/domain_conf.c | 41 +++++++++++------------------------------ src/conf/domain_conf.h | 4 +++- 2 files changed, 14 insertions(+), 31 deletions(-) -- 2.31.1

3 8

What should we do for contributing a new hypervisor driver?
by Motohiro Kawahito 12 Jul '21

12 Jul '21

Hello. This is my first time of sending e-mail to this mailing list. We are developing a new hypervisor driver. What should we prepare for contributing it? So far, we implemented the following functions. Are there any other things we should prepare? .connectOpen .connectClose .domainCreateXML .domainCreate .domainCreateWithFlags .domainShutdown .domainShutdownFlags .domainDestroy .domainDestroyFlags .connectNumOfDomains .connectListDefinedDomains .connectNumOfDefinedDomains .connectListDomains .connectListAllDomains .domainGetInfo .domainGetState .connectIsAlive .domainHasManagedSaveImage .domainGetXMLDesc .connectGetType .connectGetVersion .domainLookupByID .domainLookupByUUID .domainLookupByName .domainIsActive .domainIsPersistent .domainGetOSType .domainUndefine .domainUndefineFlags Motohiro Kawahito, Commercial Systems, IBM Research - Tokyo 19-21 Nihonbashi, Hakozaki-cho Chuo-ku, Tokyo 103-8510, Japan

2 1

[PATCH v3 0/3] Implement virDomainGetMessages for test driver
by Luke Yue 12 Jul '21

12 Jul '21

v3: - Squash tests commit - Extract the same code in test driver and qemu driver to a function Luke Yue (3): test_driver: Implement virDomainGetMessages test_driver: Introduce testDomainObjCheckTaint conf: domain: Introduce and use virDomainObjGetMessages() src/conf/domain_conf.c | 53 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 5 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 34 +----------------- src/test/test_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++ tests/virshtest.c | 2 ++ 6 files changed, 140 insertions(+), 33 deletions(-) -- 2.32.0

2 8

[PATCH 0/3] softmmu/vl: Deprecate old and crufty display options
by Thomas Huth 12 Jul '21

12 Jul '21

-display sdl uses a hand-crafted parser in vl.c, which is quite ugly since the other parts of -display have been QAPIfied already. A straight conversion to QAPI is not advisable since the "alt_grab" and "ctrl_grab" parameters are not the best solution anyway. So this patch series introduces a new "grab-mod" parameter as replacement instead and then deprecates the old and crufty options. While we're at it, the third patch also suggests to deprecated the old -sdl and -curses top-level options. Thomas Huth (3): softmmu/vl: Add a "grab-mod" parameter to the -display sdl option softmmu/vl: Deprecate the old grab options softmmu/vl: Deprecate the -sdl and -curses option docs/system/deprecated.rst | 20 ++++++++++++++++++++ qemu-options.hx | 18 +++++++++++++----- softmmu/vl.c | 24 +++++++++++++++++++++--- 3 files changed, 54 insertions(+), 8 deletions(-) -- 2.27.0

2 4

[PATCH] qemuProcessSetupDisksTransientHotplug: skip system_reset when the all disks have transient shareBacking option
by Masayoshi Mizuma 12 Jul '21

12 Jul '21

From: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com> When both <on_reboot>destroy</on_reboot/> and <transient shareBacking='yes'> is set to the domain xml, the guest doesn't start. # virsh start Guest error: Failed to start domain 'Guest' error: internal error: qemu unexpectedly closed the monitor # That's because libvirt does system_reset qmp command when <transient shareBacking='yes'> is set, and the qemu is launched with '-no-reboot' option, so system_reset goes to shutdown. The boot order issue happens only if the guest has not only the transient share backing disks but also disks which isn't set the option. Let's skip system_reset when the all disks has the option. Signed-off-by: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com> --- src/qemu/qemu_process.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2b03b0ab98..82dba64e21 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7058,7 +7058,8 @@ qemuProcessSetupDisksTransientHotplug(virDomainObj *vm, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; - bool hasHotpluggedDisk = false; + bool needReset = true; + size_t attached = 0; size_t i; for (i = 0; i < vm->def->ndisks; i++) { @@ -7071,12 +7072,24 @@ qemuProcessSetupDisksTransientHotplug(virDomainObj *vm, if (qemuDomainAttachDiskGeneric(priv->driver, vm, domdisk, asyncJob) < 0) return -1; - hasHotpluggedDisk = true; + attached++; + } + + /* system_reset is needed in case the domain also has the other disks (doesn't + * have <transient shareBacking='yes'/> option) to fix the boot order. */ + if (!attached || vm->def->ndisks == attached) + needReset = false; + + if (needReset && priv->allowReboot == VIR_TRISTATE_BOOL_NO) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Doesn't support this configuration. " + "Change the domain xml to allow rebooting the guest.")); + return -1; } /* in order to allow booting from such disks we need to issue a system-reset * so that the firmware tables recording bootable devices are regerated */ - if (hasHotpluggedDisk) { + if (needReset) { int rc; if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) -- 2.27.0

2 1

[PATCH] nodedev: fix error when no defined mdev exist
by Boris Fiuczynski 12 Jul '21

12 Jul '21

Commit e9b534905f4 introduced an error when parsing an empty list returned from mdevctl. This occurs e.g. if nodedev-undefine is used to undefine the last defined mdev which cuases the following error messages libvirtd[33143]: internal error: Unexpected format for mdevctl response libvirtd[33143]: internal error: failed to query mdevs from mdevctl: libvirtd[33143]: mdevctl failed to updated mediated devices Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/node_device/node_device_driver.c | 68 ++++++++++--------- .../mdevctl-list-empty.json | 1 + .../mdevctl-list-empty.out.xml | 0 tests/nodedevmdevctltest.c | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-empty.out.xml diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b4dd57e5f4..31525f2312 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1123,51 +1123,53 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, /* mdevctl list --dumpjson produces an output that is an array that * contains only a single object which contains a property for each parent * device */ - if (virJSONValueArraySize(json_devicelist) != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected format for mdevctl response")); - goto error; - } - - obj = virJSONValueArrayGet(json_devicelist, 0); - - if (!virJSONValueIsObject(obj)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("device list is not an object")); - goto error; - } - - n = virJSONValueObjectKeysNumber(obj); - for (i = 0; i < n; i++) { - const char *parent; - virJSONValue *child_array; - int nchildren; + if (virJSONValueArraySize(json_devicelist) > 0) { + if (virJSONValueArraySize(json_devicelist) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected format for mdevctl response")); + goto error; + } - /* The key of each object property is the name of a parent device - * which maps to an array of child devices */ - parent = virJSONValueObjectGetKey(obj, i); - child_array = virJSONValueObjectGetValue(obj, i); + obj = virJSONValueArrayGet(json_devicelist, 0); - if (!virJSONValueIsArray(child_array)) { + if (!virJSONValueIsObject(obj)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Parent device's JSON object data is not an array")); + _("device list is not an object")); goto error; } - nchildren = virJSONValueArraySize(child_array); + n = virJSONValueObjectKeysNumber(obj); + for (i = 0; i < n; i++) { + const char *parent; + virJSONValue *child_array; + int nchildren; - for (j = 0; j < nchildren; j++) { - g_autoptr(virNodeDeviceDef) child = NULL; - virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + /* The key of each object property is the name of a parent device + * which maps to an array of child devices */ + parent = virJSONValueObjectGetKey(obj, i); + child_array = virJSONValueObjectGetValue(obj, i); - if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + if (!virJSONValueIsArray(child_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to parse child device")); + _("Parent device's JSON object data is not an array")); goto error; } - if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + nchildren = virJSONValueArraySize(child_array); + + for (j = 0; j < nchildren; j++) { + g_autoptr(virNodeDeviceDef) child = NULL; + virJSONValue *child_obj = virJSONValueArrayGet(child_array, j); + + if (!(child = nodeDeviceParseMdevctlChildDevice(parent, child_obj))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to parse child device")); + goto error; + } + + if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) + goto error; + } } } diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.json b/tests/nodedevmdevctldata/mdevctl-list-empty.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-list-empty.json @@ -0,0 +1 @@ +[] diff --git a/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml b/tests/nodedevmdevctldata/mdevctl-list-empty.out.xml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index 8ba1d2da70..e246de4d87 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -360,6 +360,7 @@ mymain(void) DO_TEST_LIST_DEFINED(); + DO_TEST_PARSE_JSON("mdevctl-list-empty"); DO_TEST_PARSE_JSON("mdevctl-list-multiple"); DO_TEST_DEFINE("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); -- 2.31.1

2 4

Requesting a release of ruby-libvirt gem
by Darragh Bailey 11 Jul '21

11 Jul '21

Hi, Would it be possible to get a new release of the ruby-libvirt gem pushed to rubygems? Currently with the latest vagrant moving to ruby 3.0, bumping into an issue with ruby-libvirt having problems being installed as a dependency of vagrant-libvirt: https://github.com/vagrant-libvirt/vagrant-libvirt/issues/1322 https://github.com/hashicorp/vagrant/issues/12445 The latest master contains a fix for the include path of the st.h file from ruby includes. There is a workaround mentioned at https://github.com/hashicorp/vagrant/issues/12445#issuecomment-876566065 using: CFLAGS="-I/opt/vagrant/embedded/include/ruby-3.0.0/ruby" vagrant plugin install vagrant-libvirt I've done a test using rvm with both ruby 2.6.6 and 3.0.0 to check building a local package of the latest tip of ruby-libvirt can be installed correctly for both ruby 3.0 and earlier so it shouldn't break with slightly older versions of vagrant. -- Darragh Bailey

1 0

[PATCH 0/2] qemu: Do not erase duplicate input from namespace
by Kristina Hanicova 09 Jul '21

09 Jul '21

If the attempt to attach an input device failed, we erased the unattached device from the namespace. This resulted in erasing an already attached device in case of a duplicate. We need to check for existing file in the namespace in order to determine erasing it in case of a failure. Kristina Hanicova (2): qemu: Check for existing file in namespace qemu: Do not erase input device from namespace if duplicate src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_namespace.c | 34 ++++++++++++++++++++++------------ src/qemu/qemu_namespace.h | 3 ++- src/util/virprocess.c | 6 ++++-- 4 files changed, 29 insertions(+), 17 deletions(-) -- 2.31.1

3 5