January 2018 - Devel - libvirt List Archives

[libvirt] [PATCH] util: use union for sockaddr structs to avoid aliasing
by Daniel P. Berrangé 31 Jan '18

31 Jan '18

From: "Daniel P. Berrange" <berrange(a)redhat.com> Some platforms/toolchains will complain about casting sockaddr_storage to sockaddr_un because it breaks strict aliasing rule ../../src/util/virutil.c: In function 'virGetUNIXSocketPath': ../../src/util/virutil.c:2005: error: dereferencing pointer 'un' does break strict-aliasing rules [-Wstrict-aliasing] Change the code to use a union, in the same way that the virsocketaddr.h header does. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- Pushed as (yet another) CI build fix src/util/virutil.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index c23fa8f92e..cd6fbf2f30 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1985,30 +1985,33 @@ virGetListenFDs(void) #ifdef HAVE_SYS_UN_H char *virGetUNIXSocketPath(int fd) { - struct sockaddr_storage ss = { 0 }; - struct sockaddr_un *un = (struct sockaddr_un *)&ss; - socklen_t len = sizeof(ss); + union { + struct sockaddr sa; + struct sockaddr_storage ss; + struct sockaddr_un un; + } addr = { .ss = { 0 } }; + socklen_t len = sizeof(addr.ss); char *path; - if (getsockname(fd, (struct sockaddr *)&ss, &len) < 0) { + if (getsockname(fd, &addr.sa, &len) < 0) { virReportSystemError(errno, _("Unable to get address of FD %d"), fd); return NULL; } - if (ss.ss_family != AF_UNIX) { + if (addr.ss.ss_family != AF_UNIX) { virReportSystemError(EINVAL, _("FD %d is not a UNIX socket, has af=%d"), - fd, ss.ss_family); + fd, addr.ss.ss_family); return NULL; } - if (un->sun_path[0] == '\0') - un->sun_path[0] = '@'; + if (addr.un.sun_path[0] == '\0') + addr.un.sun_path[0] = '@'; - if (VIR_ALLOC_N(path, sizeof(un->sun_path) + 1) < 0) + if (VIR_ALLOC_N(path, sizeof(addr.un.sun_path) + 1) < 0) return NULL; - memcpy(path, un->sun_path, sizeof(un->sun_path)); - path[sizeof(un->sun_path)] = '\0'; + memcpy(path, addr.un.sun_path, sizeof(addr.un.sun_path)); + path[sizeof(addr.un.sun_path)] = '\0'; return path; } -- 2.14.3

1 0

[libvirt] [PATCH] Fixed virGetUNIXSocketPath stub on Win32
by Daniel P. Berrangé 31 Jan '18

31 Jan '18

The _() macro was not terminated and an argument needs to be marked as unused. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a win32 build fix src/util/virutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 8352e0f1f1..c23fa8f92e 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2014,10 +2014,10 @@ char *virGetUNIXSocketPath(int fd) #else /* HAVE_SYS_UN_H */ -char *virGetUNIXSocketPath(int fd) +char *virGetUNIXSocketPath(int fd ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", - _("UNIX sockets not supported on this platform"); + _("UNIX sockets not supported on this platform")); return NULL; } -- 2.14.3

1 0

[libvirt] [PATCH 0/9] Set the SCSI controller model during post parse
by John Ferlan 31 Jan '18

31 Jan '18

Fallout or a pile yak shavings from the series to move the controller validation from command line building into domain xml validation: https://www.redhat.com/archives/libvir-list/2018-January/msg00188.html This series grabs the first patch from the other series that was essentially already ACK'd, but instead of heading down the path of "working around" the fact that SCSI controller model may not be set after domain post processing, this series builds up a series of changes to implement altering the SCSI controller model during device post processing rather than waiting for the validation phase to "cheat" and alter a local model value temporarily. The first 4 patches are relatively straightforward and don't change any of the outputs. Starting with patch 5, things get a bit more interesting. Patch 6 is where the conversion to set the default model for SCSI controllers starts... Patch 7 is where things got a bit tricky w/r/t the implicit controller... Patches 8 and 9 just perform cleanup from that setting. If this is accepted - I'll go back to the other series to adjust and repost; otherwise, we can determine whether the other series is necessary or if we're just happy with the way things are. John Ferlan (9): qemu: Split qemuDomainSetSCSIControllerModel conf: Rework and rename virDomainDeviceFindControllerModel qemu: Introduce qemuDomainFindSCSIControllerModel qemu: Introduce qemuDomainGetSCSIControllerModel qemu: Fetch/save the default SCSI controller model during hotplug qemu: Introduce qemuDomainSetSCSIControllerModel conf: Allow configuration of implicit controller model qemu: Reduce need to call qemuDomainGetSCSIControllerModel qemu: Update qemuDomainFindSCSIControllerModel return src/conf/domain_conf.c | 41 ++++-- src/conf/domain_conf.h | 6 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_alias.c | 14 +- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_command.c | 88 +++++++++--- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 8 +- src/qemu/qemu_domain_address.c | 149 +++++++++++---------- src/qemu/qemu_domain_address.h | 11 +- src/qemu/qemu_hotplug.c | 14 +- src/vbox/vbox_common.c | 8 +- tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +- tests/qemuargv2xmldata/pseries-disk.xml | 2 +- .../qemuhotplug-base-with-scsi-controller-live.xml | 8 +- ...se-without-scsi-controller-live+disk-scsi-2.xml | 8 +- tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +- .../hostdev-scsi-lsi-iscsi-auth.xml | 2 +- .../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +- tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +- 20 files changed, 229 insertions(+), 146 deletions(-) -- 2.13.6

2 11

[libvirt] [PATCH] virUSBDeviceNew: Construct vroot path properly
by Michal Privoznik 31 Jan '18

31 Jan '18

When starting an LXC container, the /dev entries are created under temp root (/var/run/libvirt/lxc/$name.dev), relabelled and then the root is pivoted. However, when it comes to USB devices which keep path to the device in the structure we need a way to override the default /dev/usb/... path because we want to work with the one under temp root. That's what @vroot argument is for in virUSBDeviceNew. However, what is being passed there is: vroot = /var/run/libvirt/lxc/lxc_0.dev/bus/usb Therefore, constructed path is wrong: dev->path = //var/run/libvirt/lxc/lxc_0.dev/bus/usb//dev/bus/usb/002/002 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virusb.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/util/virusb.c b/src/util/virusb.c index 6359235ff..f06639eec 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -343,9 +343,13 @@ virUSBDeviceNew(unsigned int bus, virUSBDeviceFree(dev); return NULL; } - if (virAsprintf(&dev->path, "%s" USB_DEVFS "%03d/%03d", - vroot ? vroot : "", - dev->bus, dev->dev) < 0) { + + if ((vroot && + virAsprintf(&dev->path, "%s/%03d/%03d", + vroot, dev->bus, dev->dev) < 0) || + (!vroot && + virAsprintf(&dev->path, USB_DEVFS "%03d/%03d", + dev->bus, dev->dev) < 0)) { virUSBDeviceFree(dev); return NULL; } -- 2.13.6

2 1

[libvirt] [PATCH] blockjob: Fix a error checking of blockjob status in some case
by Jie Wang 31 Jan '18

31 Jan '18

offset and len can also be equal to 0 on failed if blockjob return status:"BLOCK_JOB_COMPLETED" with error:"File descriptor in bad state", this bug can be reproduced easily if you perform a async block job on read only filesystem, so it's very worthwhile for us to check 'error' in this case. --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5ddd855..442b218 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -889,7 +889,7 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon, case VIR_DOMAIN_BLOCK_JOB_COMPLETED: error = virJSONValueObjectGetString(data, "error"); /* Make sure the whole device has been processed */ - if (offset != len) + if (offset != len || error) event = VIR_DOMAIN_BLOCK_JOB_FAILED; break; case VIR_DOMAIN_BLOCK_JOB_CANCELED: -- 1.8.3.1

3 2

[libvirt] [PATCH 0/3] tools: Actually enable bash-completion
by Michal Privoznik 31 Jan '18

31 Jan '18

Due to some naming convention the bash script for inactive virsh/virt-admin wasn't loaded. Fix this. At the same time, fix two small nits raised in the other patch set for bash completion. Michal Privoznik (3): virshDomainNameCompleter: Prune accepted flags virsh: Offer only persistent domains for autostart tools: Make symlinks to vsh bash-completion script libvirt.spec.in | 24 +++++++++++++++++++++++- tools/Makefile.am | 7 ++++++- tools/virsh-completer.c | 13 +++---------- tools/virsh-domain.c | 2 +- 4 files changed, 33 insertions(+), 13 deletions(-) -- 2.13.6

3 10

[libvirt] [PATCH] resctl: stub out functions with Linux-only APIs used
by Daniel P. Berrangé 31 Jan '18

31 Jan '18

The flock() function and d_type field in struct dirent are not portable to the mingw platform. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- * Pushed as CI build fix src/util/virresctrl.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index e252aefe31..754820ee46 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -295,6 +295,7 @@ virResctrlAllocNew(void) /* Common functions */ +#ifdef __linux__ static int virResctrlLockInternal(int op) { @@ -321,6 +322,20 @@ virResctrlLockWrite(void) return virResctrlLockInternal(LOCK_EX); } +#else + +static inline int +virResctrlLockWrite(void) +{ + virReportSystemError(ENOSYS, "%s", + _("resctrlfs not supported on this platform")); + return -1; +} + +#endif + + + static int virResctrlUnlock(int fd) @@ -328,6 +343,7 @@ virResctrlUnlock(int fd) if (fd == -1) return 0; +#ifdef __linux__ /* The lock gets unlocked by closing the fd, which we need to do anyway in * order to clean up properly */ if (VIR_CLOSE(fd) < 0) { @@ -338,6 +354,7 @@ virResctrlUnlock(int fd) virReportSystemError(errno, "%s", _("Cannot unlock resctrlfs")); return -1; } +#endif /* ! __linux__ */ return 0; } @@ -369,6 +386,8 @@ virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) } +#ifdef __linux__ + int virResctrlGetInfo(virResctrlInfoPtr resctrl) { @@ -495,6 +514,18 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) return ret; } +#else /* ! __linux__ */ + +int +virResctrlGetInfo(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Cache tune not supported on this platform")); + return -1; +} + +#endif /* ! __linux__ */ + int virResctrlInfoGetCache(virResctrlInfoPtr resctrl, @@ -632,6 +663,8 @@ virResctrlAllocGetType(virResctrlAllocPtr resctrl, } +#ifdef __linux__ + static int virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, unsigned int level, @@ -659,6 +692,8 @@ virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, return virBitmapCopy(a_type->masks[cache], mask); } +#endif + static int virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, @@ -878,6 +913,8 @@ virResctrlAllocFormat(virResctrlAllocPtr resctrl) } +#ifdef __linux__ + static int virResctrlAllocParseProcessCache(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, @@ -1180,7 +1217,17 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) goto cleanup; } +#else /* ! __linux__ */ + +virResctrlAllocPtr +virResctrlAllocGetUnused(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Cache tune not supported on this platform")); + return NULL; +} +#endif /* ! __linux__ */ static int virResctrlAllocSetMask(virResctrlAllocPerTypePtr a_type, -- 2.14.3

2 3

[libvirt] [PATCH 0/7] Last CAT fixups
by Martin Kletzander 31 Jan '18

31 Jan '18

Few finishing touches for the Cache Allocation Technology. This series copies the default group's mask to newly created groups, fixes a possible wrong memory access, cleans up after domains even if daemon was restarted, and then reorders the functions in virresctrl.c so that it's nice to find the non-Linux stubs and Linux-only functions. And the last thing is just one variable rename that was forgotten before. Martin Kletzander (8): util: Add helpers for getting resctrl group allocs util: Use default group's mask for unspecified resctrl allocations util: Don't overwrite mask in virResctrlAllocFindUnused qemu: Restore machinename even without cgroups util: Extract path formatting into virResctrlAllocDeterminePath qemu: Restore resctrl alloc data after restart util: Reorder functions in virresctrl to minimize stubs util: Rename resctrl to alloc in virResctrlAllocUpdateMask to be consistent src/libvirt_linux.syms | 3 + src/libvirt_private.syms | 2 +- src/qemu/qemu_cgroup.c | 4 - src/qemu/qemu_process.c | 11 + src/util/virresctrl.c | 1620 +++++++++++++++++++++++----------------------- src/util/virresctrl.h | 4 + 6 files changed, 843 insertions(+), 801 deletions(-) -- 2.16.1

2 11

[libvirt] [PATCH go-xml] Add reconnect support for vhost-user interfaces
by Andrea Bolognani 31 Jan '18

31 Jan '18

Introduced in libvirt commit 614be3b88276. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Does this look like Go to you? Because I have no idea what I'm doing. domain.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/domain.go b/domain.go index 429e61b..4f7725a 100644 --- a/domain.go +++ b/domain.go @@ -392,9 +392,10 @@ type DomainInterfaceSourceEthernet struct { } type DomainInterfaceSourceVHostUser struct { - Type string `xml:"type,attr"` - Path string `xml:"path,attr,omitempty"` - Mode string `xml:"mode,attr,omitempty"` + Type string `xml:"type,attr"` + Path string `xml:"path,attr,omitempty"` + Mode string `xml:"mode,attr,omitempty"` + Reconnect *DomainInterfaceSourceReconnect `xml:"reconnect"` } type DomainInterfaceSourceServer struct { @@ -449,6 +450,11 @@ type DomainInterfaceSourceLocal struct { Port uint `xml:"port,attr,omitempty"` } +type DomainInterfaceSourceReconnect struct { + Enabled string `xml:"enabled,attr"` + Timeout *uint `xml:"timeout,attr"` +} + type DomainInterfaceTarget struct { Dev string `xml:"dev,attr"` } -- 2.14.3

2 1

[libvirt] [PATCH v2] vbox: fix SEGV during dumpxml of a serial port
by Laine Stump 31 Jan '18

31 Jan '18

commit 77a12987a48 changed the "virDomainChrSourceDef source" inside virDomainChrDef to "virDomainChrSourceDefPtr source", and started allocating source inside virDomainChrDefNew(), but vboxDumpSerial() was allocating a virDomainChrDef with a simple VIR_ALLOC() (i.e. never calling virDomainChrDefNew()), so source was never initialized, leading to a SEGV any time a serial port was present. The same problem was created in vboxDumpParallel(). This patch changes vboxDumpSerial() and vboxDumpParallel() to use virDomainChrDefNew() instead of VIR_ALLOC(), and changes both of those functions to return an error if virDomainChrDef() (or any other allocation) fails. This resolves: https://bugzilla.redhat.com/1536649 --- Change from V1: Due to reviews of V1 whining about the patch calling out the lack of returning failure on OOM in a comment rather than fixing it, V2 changes both functions to return 0/-1, and their caller to abort the dumpxml if they return -1. (If I'd taken the extra 30 seconds to look one level up the call chain before posting V1, I would have done that to begin with - there's really nothing complicated about the change, so I'm just as comfortable posting this V2 patch without being able to test as I was with V1). src/vbox/vbox_common.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 33aefabe5..e8381ac95 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3862,7 +3862,7 @@ vboxDumpAudio(virDomainDefPtr def, vboxDriverPtr data ATTRIBUTE_UNUSED, } } -static void +static int vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 serialPortCount) { PRUint32 serialPortIncCount = 0; @@ -3886,9 +3886,15 @@ vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUin } /* Allocate memory for the serial ports which are enabled */ - if ((def->nserials > 0) && (VIR_ALLOC_N(def->serials, def->nserials) >= 0)) { - for (i = 0; i < def->nserials; i++) - ignore_value(VIR_ALLOC(def->serials[i])); + if (def->nserials > 0) { + if (VIR_ALLOC_N(def->serials, def->nserials) < 0) + return -1; + + for (i = 0; i < def->nserials; i++) { + def->serials[i] = virDomainChrDefNew(NULL); + if (!def->serials[i]) + return -1; + } } /* Now get the details about the serial ports here */ @@ -3936,7 +3942,8 @@ vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUin if (pathUtf16) { VBOX_UTF16_TO_UTF8(pathUtf16, &path); - ignore_value(VIR_STRDUP(def->serials[serialPortIncCount]->source->data.file.path, path)); + if (VIR_STRDUP(def->serials[serialPortIncCount]->source->data.file.path, path) < 0) + return -1; } serialPortIncCount++; @@ -3948,9 +3955,10 @@ vboxDumpSerial(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUin VBOX_RELEASE(serialPort); } } + return 0; } -static void +static int vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 parallelPortCount) { PRUint32 parallelPortIncCount = 0; @@ -3974,9 +3982,15 @@ vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU } /* Allocate memory for the parallel ports which are enabled */ - if ((def->nparallels > 0) && (VIR_ALLOC_N(def->parallels, def->nparallels) >= 0)) { - for (i = 0; i < def->nparallels; i++) - ignore_value(VIR_ALLOC(def->parallels[i])); + if (def->nparallels > 0) { + if (VIR_ALLOC_N(def->parallels, def->nparallels) < 0) + return -1; + + for (i = 0; i < def->nparallels; i++) { + def->parallels[i] = virDomainChrDefNew(NULL); + if (!def->parallels[i]) + return -1; + } } /* Now get the details about the parallel ports here */ @@ -4011,7 +4025,8 @@ vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU gVBoxAPI.UIParallelPort.GetPath(parallelPort, &pathUtf16); VBOX_UTF16_TO_UTF8(pathUtf16, &path); - ignore_value(VIR_STRDUP(def->parallels[parallelPortIncCount]->source->data.file.path, path)); + if (VIR_STRDUP(def->parallels[parallelPortIncCount]->source->data.file.path, path) < 0) + return -1; parallelPortIncCount++; @@ -4022,6 +4037,7 @@ vboxDumpParallel(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU VBOX_RELEASE(parallelPort); } } + return 0; } static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) @@ -4162,8 +4178,11 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) vboxDumpSharedFolders(def, data, machine); vboxDumpNetwork(def, data, machine, networkAdapterCount); vboxDumpAudio(def, data, machine); - vboxDumpSerial(def, data, machine, serialPortCount); - vboxDumpParallel(def, data, machine, parallelPortCount); + + if (vboxDumpSerial(def, data, machine, serialPortCount) < 0) + goto cleanup; + if (vboxDumpParallel(def, data, machine, parallelPortCount) < 0) + goto cleanup; /* dump USB devices/filters if active */ vboxHostDeviceGetXMLDesc(data, def, machine); -- 2.14.3

2 1