- Devel - libvirt List Archives

[libvirt] libvirt API/design questions
by Cole Robinson 22 Dec '19

22 Dec '19

There's some pre-existing libvirt design questions I would like some feedback on, and one new one * `virsh blockresize` doesn't prevent shrink by default, couple users have blown their foot off and there's attempts at patches. https://www.redhat.com/archives/libvir-list/2019-October/msg00258.html https://www.redhat.com/archives/libvir-list/2019-November/msg00843.html Do we implement this as a protection in virsh, or change the API behavior and require a new API flag to allow shrinking? Or some other idea? * vhost-user-scsi and vhost-user-blk XML schema https://www.redhat.com/archives/libvirt-users/2019-October/msg00018.html Last proposal was using <hostdev> for this, which revisiting it now makes more sense than <disk>, because it vhost-user-X doesn't seem to use qemu's block layer at all. So vhost-user-scsi would be: <hostdev mode='subsystem' type='scsi_host'> <source protocol='vhostuser' type='unix' path='/path/to/vhost-user-scsi.sock' mode='client'/> </hostdev> vhost-user-blk maybe requires a new type='block' ? Otherwise it's unclear how to differentiate between the two. Regardless it would be nice to give the original patch author guidance here, they seemed motivated to get this working * Splitting domain_conf up. There's 30k lines in domain_conf.c and 3500 in domain_conf.h, it would be nice to unwind it a bit. Getting some sign off on this ahead of time is critical IMO so pieces can be posted and committed quickly because they will obviously go out of date fast. My thoughts: - domain_def.[ch]: DomainDefXXX and enum definitions. - probably New and Free functions too - domain_parse.[ch]: XML parsing - domain_format.[ch]: XML formatting - domain_validate.[ch]: validate and postparse handling - domain_util.[ch]: everything else, or keep it in domain_conf? domain_def should be easy but no idea how intertwined the rest are. If the domain_validate naming is acceptable for both validate and postparse functions, we could use that naming for qemu_domain.c split too. Also maybe it's worth considering if there's some way to sensibly split the conf/ directory. We could add a top level domain/ directory, but that's kinda ambiguously named as we already have network/ + storage/ + secret/ etc that have different meanings. Maybe sub dirs like conf/domain/ ? I'm curious if anyone has strong feelings either way. There's not really a clear place to dump shared DomainDef code at the moment, like possible domain_cgroup for sharing cgroup handling across lxc + qemu Thanks, Cole

4 5

[libvirt] virsh blockresize syntax is inconsistent with vol-resize and somewhat dangerous
by Tim Small 22 Dec '19

22 Dec '19

Hello, virsh has two commands which can be used to resize block devices - "blockresize" for volumes in use by and active guest, and "vol-resize" for volumes which are not in use. The vol-resize syntax allows to specify the size as a delta (increase or decrease vs. the current size), and also refuses to shrink a volume unless the "--shrink" argument is also passed. Most other tools which can be used for block device resizing (outside of libvirt) also have similar "--shrink" argument requirements when reducing the size of an existing block device. e.g. ceph requires "--allow-shrink" when using the "rbd resize" command. The lack of such a safety device makes "blockresize" a foot-gun (which I recently found to great effect when I typoed the domain name to another valid domain). It seems I am not alone in making this error e.g. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902171 One possible solution would be to make a new command e.g. "domblkresize" or perhaps "live-resize", which implement the "--shrink" and "--delta" behaviour to make it consistent with "vol-resize" syntax, and mark the "blockresize" command as deprecated in the documentation and help (so that existing automation which depends on the current behaviour doesn't break). Any thoughts? Should I open this as an RFE? Thanks, Tim. -- South East Open Source Solutions Limited Registered in England and Wales with company number 06134732. Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ VAT number: 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309

3 3

[libvirt] [PATCH v5 0/5] qemu_process: Glib sponsored cleanup
by Daniel Henrique Barboza 20 Dec '19

20 Dec '19

changes from previous version 4 [1]: - patch 1: * moved 'dev' inside the for loop in qemuProcessHandleBlockThreshold() * moved 'id' inside the 'for' loop in qemuProcessLookupPTYs() * moved 'path' inside both conditionals in qemuProcessBuildDestroyMemoryPaths() * set 'template=NULL' in case g_mkdtemp() succeeds in qemuProcessQMPInit() - patch 2: new patch. add g_autoptr() support for qemuDomainLogContextPtr - patch 3: * using g_autoptr() on feasible virCommandPtr and * qemuDomainLogContextPtr [1] https://www.redhat.com/archives/libvir-list/2019-December/msg01311.html Daniel Henrique Barboza (5): qemu_process.c: use g_autofree qemu_domain.h: add G_DEFINE_AUTOPTR_CLEANUP_FUNC for qemuDomainLogContext qemu_process.c: use g_autoptr() qemu_process.c: remove cleanup labels after g_auto*() changes qemu_process.c: remove 'cleanup' label from qemuProcessCreatePretendCmd() src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 445 +++++++++++++++------------------------- 2 files changed, 166 insertions(+), 280 deletions(-) -- 2.24.1

2 6

[libvirt] [PATCH v4 0/4] qemu_process: Glib sponsored cleanup
by Daniel Henrique Barboza 20 Dec '19

20 Dec '19

This series got buried a few months ago. Let's go onward unto the 20s with no one left behind. changes from version 3 [1]: - rebased to master at commit 330b556829 - removed former patch 4. The 'g_strdup_printf' change was made along the road - new patch 4: I am sending this one in separate to patch 3 because this unneeded label was already in the code, being unrelated to the changes of this series. The maintainer is welcome to squash it into patch 3. [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01080.html Daniel Henrique Barboza (4): qemu_process.c: use g_autofree qemu_process.c: use g_autoptr() qemu_process.c: remove cleanup labels after g_auto*() changes qemu_process.c: remove 'cleanup' label from qemuProcessCreatePretendCmd() src/qemu/qemu_process.c | 429 +++++++++++++++------------------------- 1 file changed, 157 insertions(+), 272 deletions(-) -- 2.23.0

3 12

[libvirt] [PATCH] schemas: Allow additional qemu cmd line arguments/env variables and qemuCaps to be interleaved
by Michal Privoznik 20 Dec '19

20 Dec '19

While command line arguments are sort of positional (becasue you have to have two entries, one for "-arg" the other for "value"), it doesn't really matter whether env variables come before or after command line arguments. And it matter even less when playing with qemu capabilities. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- docs/schemas/domaincommon.rng | 54 +++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5f1d4a34a4..b87457382a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6295,37 +6295,41 @@ --> <define name="qemucmdline"> <element name="commandline" ns="http://libvirt.org/schemas/domain/qemu/1.0"> - <zeroOrMore> - <element name="arg"> - <attribute name='value'/> - </element> - </zeroOrMore> - <zeroOrMore> - <element name="env"> - <attribute name='name'> - <ref name="filter-param-name"/> - </attribute> - <optional> + <interleave> + <zeroOrMore> + <element name="arg"> <attribute name='value'/> - </optional> - <empty/> - </element> - </zeroOrMore> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="env"> + <attribute name='name'> + <ref name="filter-param-name"/> + </attribute> + <optional> + <attribute name='value'/> + </optional> + <empty/> + </element> + </zeroOrMore> + </interleave> </element> </define> <define name="qemucapabilities"> <element name="capabilities" ns="http://libvirt.org/schemas/domain/qemu/1.0"> - <zeroOrMore> - <element name="add"> - <attribute name="capability"/> - </element> - </zeroOrMore> - <zeroOrMore> - <element name="del"> - <attribute name="capability"/> - </element> - </zeroOrMore> + <interleave> + <zeroOrMore> + <element name="add"> + <attribute name="capability"/> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="del"> + <attribute name="capability"/> + </element> + </zeroOrMore> + </interleave> </element> </define> -- 2.24.1

3 2

[libvirt] [PATCH] util: add note about event file descriptors on Windows
by Daniel P. Berrangé 20 Dec '19

20 Dec '19

When using GNULIB with Winsock, libvirt will never see the normal HANDLE objects, instead GNULIB guarantees that libvirt gets a C runtime file descriptor. The GNULIB poll impl also expects to get C runtime file descriptors rather than HANDLE objects. Document this behaviour so that it is clear to applications providing event loop implementations if they need Windows portability. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- include/libvirt/libvirt-event.h | 4 ++++ src/util/virevent.c | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/include/libvirt/libvirt-event.h b/include/libvirt/libvirt-event.h index 734dbdcbc1..facdc3a3ec 100644 --- a/include/libvirt/libvirt-event.h +++ b/include/libvirt/libvirt-event.h @@ -67,6 +67,10 @@ typedef void (*virEventHandleCallback)(int watch, int fd, int events, void *opaq * listen for specific events. The same file handle can be registered * multiple times provided the requested event sets are non-overlapping * + * @fd will always be a C runtime file descriptor. On Windows + * the _get_osfhandle() method can be used if a HANDLE is required + * instead. + * * If the opaque user data requires free'ing when the handle * is unregistered, then a 2nd callback can be supplied for * this purpose. This callback needs to be invoked from a clean stack. diff --git a/src/util/virevent.c b/src/util/virevent.c index f6c797724e..fd5d8f5bf1 100644 --- a/src/util/virevent.c +++ b/src/util/virevent.c @@ -60,6 +60,13 @@ static virEventRemoveTimeoutFunc removeTimeoutImpl; * requires that an event loop has previously been registered with * virEventRegisterImpl() or virEventRegisterDefaultImpl(). * + * @fd must always always be a C runtime file descriptor. On Windows + * if the caller only has a HANDLE, the _open_osfhandle() method can + * be used to open an associated C runtime file descriptor for use + * with this API. After opening a runtime file descriptor, CloseHandle() + * must not be used, instead close() will close the runtime file + * descriptor and its original associated HANDLE. + * * Returns -1 if the file handle cannot be registered, otherwise a handle * watch number to be used for updating and unregistering for events. */ -- 2.21.0

2 1

[libvirt] [PATCH v2 0/2] Initialize host boot time global var upfront
by Michal Privoznik 20 Dec '19

20 Dec '19

This is a v2 for: https://www.redhat.com/archives/libvir-list/2019-December/msg01045.html While technically v1 was fixed I agree with Cole's suggestion, so I'm discarding v1 and sending another version which implements his suggestion. Michal Prívozník (2): virhostuptime: Introduce virHostBootTimeInit() remote_daemon: Initialize host boot time global variable src/libvirt_private.syms | 1 + src/remote/remote_daemon.c | 9 +++++++++ src/util/virhostuptime.c | 13 ++++++++++++- src/util/virhostuptime.h | 3 +++ 4 files changed, 25 insertions(+), 1 deletion(-) -- 2.24.1

3 4

[libvirt] [PATCH v2 0/3] store binary name in capabilities
by Daniel P. Berrangé 20 Dec '19

20 Dec '19

2 5

[libvirt] [PATCH] news: document phyp removal
by Cole Robinson 20 Dec '19

20 Dec '19

Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 055353b9a5..1af57f8af0 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -99,6 +99,16 @@ Python 2 binary. Python 3 must be used instead. </description> </change> + <change> + <summary> + 'phyp' Power Hypervisor driver removed + </summary> + <description> + The 'phyp' Power Hypervisor driver has not seen active development + since 2011 and does not seem to have any real world usage. It + has now been removed. + </description> + </change> </section> </release> <release version="v5.10.0" date="2019-12-02"> -- 2.23.0

2 1

[libvirt] i want to update the disk driver from ide to virtio, but fail , why ?
by thomas 20 Dec '19

20 Dec '19

hi, ALL: my question description as follow: virsh dumpxml root-vsys_v2 > v2.xml , in the v2.xml has the following information: <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/home/root-vsys_v2.qcow2'/> <backingStore/> <target dev='hda' bus='ide'/> // i want change the 'ide' to 'virtio', but fail <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> so i create a new xml name v2-new.xml ,it content is: <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/home/root-vsys_v2.qcow2'/> <target dev='vda' bus='virtio'/> </disk> then run: virsh update-device root-vsys_v2 /root/v2-new.xml --live the result is: [root@NSG ~]# virsh update-device root-vsys_v2 /root/v2-new.xml --live error: Failed to update device from /root/v2-new.xml error: internal error: No device with bus 'virtio' and target 'vda' i read the source code for libvirt ,the log outprint at funcion: static int qemuDomainChangeDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virQEMUDriverPtr driver, bool force) { virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; virDomainDeviceDef oldDev = { .type = dev->type }; int ret = -1; if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No device with bus '%s' and target '%s'"), virDomainDiskBusTypeToString(disk->bus), disk->dst); //the failure log is here, the vm->def has disk item :dev = hda, bus=ide ,and disk->bus = virtio disk->dst=vda, goto cleanup; } //the disk->bus and disk->dst can't in the vm->def ,so the api virDomainDiskFindByBusAndDst failed oldDev.data.disk = orig_disk; if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, VIR_DOMAIN_DEVICE_ACTION_UPDATE, true) < 0) goto cleanup; if (!qemuDomainDiskChangeSupported(disk, orig_disk)) goto cleanup; if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { /* Disk source can be changed only for removable devices */ if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk source can be changed only in removable " "drives")); goto cleanup; } if (qemuDomainAttachDeviceDiskLive(driver, vm, dev, force) < 0) goto cleanup; } orig_disk->startupPolicy = dev->data.disk->startupPolicy; orig_disk->snapshot = dev->data.disk->snapshot; ret = 0; cleanup: return ret; } what reason lead the faiure ? is my v2-new.xml write error ? how to write the xml for virsh update-device ? i refer the link https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/ht… -------------------------- thanks ! thosmas.kuang

2 2