- Devel - libvirt List Archives

[libvirt] [PATCH v3 0/7] Support to use iscsi storage in domain conf
by John Ferlan 24 Jul '13

24 Jul '13

An update to yesterday's posting: https://www.redhat.com/archives/libvir-list/2013-July/msg01213.html Changes in v3 over v2 * Update patch 1 per code review * Use 'direct' instead of 'uri' and supporting documentaiton * In patch 4, use qemuAddISCSIPoolSourceHost() instead of qemuAddDiskISCSIUri. Also added a check in the function that (pooldef->source.nhost != 1) to make sure we at least have a host defined in the pool and of course that there's only one. John Ferlan (5): storage_iscsi: Reflect the default target port conf: Introduce new XML tag "mode" for disk source conf: Introduce virDomainDiskSourceIsBlockType qemu: Translate the iscsi pool/volume disk source tests: Add various network and volume definitions Osier Yang (2): conf: Ignore the volume type disk if its mode is "direct" qemu: Translate the volume type disk source before cgroup setting docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 8 ++ src/conf/domain_conf.c | 67 ++++++++++- src/conf/domain_conf.h | 26 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 20 +++- src/qemu/qemu_conf.c | 124 +++++++++++++++++---- src/qemu/qemu_process.c | 13 ++- src/storage/storage_backend_iscsi.c | 19 ++-- .../qemuxml2argv-disk-source-pool-mode.xml | 48 ++++++++ tests/qemuxml2xmltest.c | 1 + tests/securityselinuxlabeldata/netdisks.txt | 5 + tests/securityselinuxlabeldata/netdisks.xml | 58 ++++++++++ tests/securityselinuxlabeldata/voldisks.txt | 5 + tests/securityselinuxlabeldata/voldisks.xml | 45 ++++++++ tests/securityselinuxlabeltest.c | 2 + 16 files changed, 414 insertions(+), 39 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml create mode 100644 tests/securityselinuxlabeldata/netdisks.txt create mode 100644 tests/securityselinuxlabeldata/netdisks.xml create mode 100644 tests/securityselinuxlabeldata/voldisks.txt create mode 100644 tests/securityselinuxlabeldata/voldisks.xml -- 1.8.1.4

4 41

[libvirt] [PATCH] dbus: work with older dbus
by Eric Blake 24 Jul '13

24 Jul '13

dbus 1.2.24 (on RHEL 6) lacks DBUS_TYPE_UNIX_FD; but as we aren't trying to pass one of those anyways, we can just drop support for it in our wrapper. Solves this build error: CC libvirt_util_la-virdbus.lo util/virdbus.c:242: error: 'DBUS_TYPE_UNIX_FD' undeclared here (not in a function) * src/util/virdbus.c (virDBusBasicTypes): Drop support for unix fds. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. src/util/virdbus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 9b0977a..4ce6c46 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1,7 +1,7 @@ /* * virdbus.c: helper for using DBus * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -239,7 +239,6 @@ static const char virDBusBasicTypes[] = { DBUS_TYPE_STRING, DBUS_TYPE_OBJECT_PATH, DBUS_TYPE_SIGNATURE, - DBUS_TYPE_UNIX_FD }; static bool virDBusIsBasicType(char c) { @@ -1024,6 +1023,9 @@ int virDBusMessageDecode(DBusMessage* msg, * '{' - start of a dictionary entry (pair of types) * '}' - start of a dictionary entry (pair of types) * + * At this time, there is no support for Unix fd's ('h'), which only + * newer DBus supports. + * * Passing values in variadic args for basic types is * simple, the value is just passed directly using the * corresponding C type listed against the type code -- 1.8.3.1

2 1

[libvirt] [PATCH 0/5] Support for pcie-root / Q35 machine type
by Laine Stump 24 Jul '13

24 Jul '13

This is *almost* usable. I still need to add support for a dmi-to-pci-bridge before the q35 machine type can be used. 1/5 and 3/5 are just cleaning up some things that bothered me while writing patch 4/5. 2/5 should help out other machine types (e.g. arm) just a bit, by eliminating the extra PIIX3 devices unless the machine actually has a PIIX3 chip. 4/5 has the bulk of the changes - it restructures the PCI address validation/allocation so that it doesn't assume that all PCI addresses are the same, or that all PCI slots are the same. It sets up an enum where different types of PCI addresses can be listed, and the validation code checks that the slot about to be used for a device is actually compatible with that device. 5/5 adds the pcie-root controller which is implicit in the q35 machinetype. Of course since we can only connect pcie devices to this controller, and there are no pcie devices supported, it is not really worth much. However, the next patch (in progress) will add a dmi-to-pci-bridge controller, which *can* plug into the pcie-root and provide plain PCI slots, making a working a35 possible. Note that the auto-allocation doesn't attempt to auto-allocate a slot for any type of device other than plain PCI. This means that even when the dmi-to-pci-bridge controller is added, its address on pcie-root (and a pci-bridge device that must be connected to the dmi-to-pci-bridge controller) will need to be manually specified. Auto-placement will be an exercise for later. Things missing: 1) I need a test case for auto-adding multiple bridges. I will add that this afternoon. 2) I need a test case for a q35 domain. That wil also be added this afternoon. Laine Stump (5): qemu: turn qemuDomainPCIAddressBus into a struct qemu: only check for PIIX3-specific device addrs on pc-* machinetypes qemu: make QEMU_PCI_ADDRESS_(SLOT|FUNCTION)_LAST less misleading qemu: set/validate slot/connection type when assigning slots for PCI devices conf: add pcie-root controller docs/formatdomain.html.in | 17 +- src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 5 +- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 559 ++++++++++++++++++++++++++++++---------------- src/qemu/qemu_command.h | 28 ++- src/qemu/qemu_domain.c | 23 +- 7 files changed, 434 insertions(+), 203 deletions(-) -- 1.7.11.7

3 16

[libvirt] [PATCH 0/2] Integration with systemd-machined
by Daniel P. Berrange 24 Jul '13

24 Jul '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> This includes a v2 of a previously submitted patch https://www.redhat.com/archives/libvir-list/2013-July/msg01084.html This posting includes a second patch to actually wrap the systemd call Changes since that first posting - Use 'int' instead of 'size_t' for the ARRAY var-arg, since a plain number "3" does not promote to 'size_t' when passed through variadic args, only to 'int'. Using 'int' thus avoids need to cast every time. - Handle DBus error replies - Make sure building --without-dbus works Daniel P. Berrange (2): Introduce virDBusCallMethod & virDBusMessageRead methods Add API for calling systemd-machined's DBus API .gitignore | 2 + include/libvirt/virterror.h | 2 + src/Makefile.am | 1 + src/libvirt_private.syms | 8 + src/util/virdbus.c | 966 +++++++++++++++++++++++++++++++++++++++++++- src/util/virdbus.h | 11 + src/util/virdbuspriv.h | 45 +++ src/util/virerror.c | 7 + src/util/virerror.h | 11 + src/util/virsystemd.c | 139 +++++++ src/util/virsystemd.h | 36 ++ tests/Makefile.am | 29 ++ tests/testutils.h | 2 + tests/virdbustest.c | 389 ++++++++++++++++++ tests/virsystemdmock.c | 77 ++++ tests/virsystemdtest.c | 131 ++++++ 16 files changed, 1855 insertions(+), 1 deletion(-) create mode 100644 src/util/virdbuspriv.h create mode 100644 src/util/virsystemd.c create mode 100644 src/util/virsystemd.h create mode 100644 tests/virdbustest.c create mode 100644 tests/virsystemdmock.c create mode 100644 tests/virsystemdtest.c -- 1.8.1.4

2 8

[libvirt] [PATCH] Don't overwrite errors in qemuTranslateDiskSourcePool
by Ján Tomko 24 Jul '13

24 Jul '13

Both virStoragePoolFree and virStorageVolFree reset the last error, which might lead to the cryptic message: An error occurred, but the cause is unknown When the volume wasn't found, virStorageVolFree was called with NULL, leading to an error: invalid storage volume pointer in virStorageVolFree This patch changes it to: Storage volume not found: no storage vol with matching name 'tomato' --- src/qemu/qemu_conf.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3e7b78a..80b8156 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1248,6 +1248,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, char *poolxml = NULL; virStorageVolInfo info; int ret = -1; + virErrorPtr savedError; if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) return 0; @@ -1324,8 +1325,14 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def->srcpool->voltype = info.type; ret = 0; cleanup: - virStoragePoolFree(pool); - virStorageVolFree(vol); + savedError = virSaveLastError(); + if (pool) + virStoragePoolFree(pool); + if (vol) + virStorageVolFree(vol); + virSetError(savedError); + virFreeError(savedError); + VIR_FREE(poolxml); virStoragePoolDefFree(pooldef); return ret; -- 1.8.1.5

2 1

Re: [libvirt] [Qemu-devel] [Question] why x2apic's set by default without host support(on Nehalem CPU).
by Eduardo Habkost 24 Jul '13

24 Jul '13

On Mon, Jul 22, 2013 at 07:24:24PM +0800, Peter Huang(Peng) wrote: > Hi, Everyone > > I have been encountered a problem recently. > My box uses Nehalem E5520 CPU, and this model doesn't support x2apic feature. > I created a VM on KVM, and uses cpu mode=pass-through, I can get the right cpu > model from the VM, but I can also see that the CPU in the vm supports x2apic. > > This seems werid. > From QEMU's CPU model definition, Nehalem also doesn't support x2apic. > > Can anyone explain it? > libvirt's "host-passthrough" uses "-cpu host', and it "-cpu host" enables every feature that can be enabled on the host. As KVM x2apic emulation doesn't require host CPU support, it gets enabled. If the behavior of "-cpu host" doesn't match libvirt expectations, we need to clarify what are the requirements, and maybe have a "try to be close to host CPU mode" as opposed to the current "enable everything that can be enabled" mode. -- Eduardo

3 4

[libvirt] [PATCH] build: work around broken kernel headers
by Eric Blake 24 Jul '13

24 Jul '13

Thanks to a lack of coordination between kernel and glibc folks, it has been impossible to mix code using <linux/in.h> and <net/in.h> for some time now. On at least RHEL 6, <linux/if_bridge.h> tries to use the kernel side, and fails due to our desire to use the glibc side elsewhere: In file included from /usr/include/linux/if_bridge.h:17, from util/virnetdevbridge.c:42: /usr/include/linux/in6.h:31: error: redefinition of ‘struct in6_addr’ /usr/include/linux/in6.h:48: error: redefinition of ‘struct sockaddr_in6’ /usr/include/linux/in6.h:56: error: redefinition of ‘struct ipv6_mreq’ Thankfully, the kernel layout of these structs is ABI-compatible, they only differ in the type system presented to the C compiler. While there are other versions of kernel headers that avoid the problem, it is easier to just work around the issue than to expect all developers to upgrade to working kernel headers. * src/util/virnetdevbridge.c (includes): Coerce the kernel version of in.h to not collide with the normal version. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Tested on RHEL 6.4 and Fedora 19, and pushing under the build-breaker rule. src/util/virnetdevbridge.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index ae9276c..5d52e23 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -39,7 +39,18 @@ #ifdef __linux__ # include <linux/sockios.h> # include <linux/param.h> /* HZ */ +/* Depending on the version of kernel vs. glibc, there may be a collision + * between <net/in.h> and kernel IPv6 structures. The different types + * are ABI compatible, but choke the C type system; work around it by + * using temporary redefinitions. */ +# define in6_addr in6_addr_ +# define sockaddr_in6 sockaddr_in6_ +# define ipv6_mreq ipv6_mreq_ +# include <linux/in6.h> # include <linux/if_bridge.h> /* SYSFS_BRIDGE_ATTR */ +# undef in6_addr +# undef sockaddr_in6 +# undef ipv6_mreq # define JIFFIES_TO_MS(j) (((j)*1000)/HZ) # define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000) -- 1.7.1

1 0

[libvirt] [PATCH 0/3] Miscellaneous Coverity & Valgrind changes
by John Ferlan 24 Jul '13

24 Jul '13

These patches resolve a Coverity warning and a found Valgrind leak. Added more patterns to the .valgrind.supp file to suppress the commandtests and seclabeltest output. This will thus result in a mostly clean valgrind run. All that remains is a hotplugtest failure, which some patches were posted: https://www.redhat.com/archives/libvir-list/2013-July/msg01156.html and https://www.redhat.com/archives/libvir-list/2013-July/msg01158.html but did not completely resolve the failures. John Ferlan (3): lxc: Resolve Coverity warning domain_event: Resolve memory leak found by Valgrind valgrind.supp: Add more valgrind suppression paths src/conf/domain_event.c | 3 +++ src/lxc/lxc_process.c | 2 +- tests/.valgrind.supp | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) -- 1.8.1.4

3 8

[libvirt] [PATCH V3] add console support in libxl
by Bamvor Jian Zhang 24 Jul '13

24 Jul '13

this patch introduce the console api in libxl driver for both pv and hvm guest. and import and update the libxlMakeChrdevStr function which was deleted in commit dfa1e1dd. Signed-off-by: Bamvor Jian Zhang <bjzhang(a)suse.com> --- Changes since V2: 1), forbid parallel configure because libxl do not support it 2), only support one serial on libxl driver. 3), also remove console code in libxl driver, AFAICS serial is enough for connecting to libxl console. changes since V1: 1), add virDomainOpenConsoleEnsureACL 3), remove virReportOOMErrorFull when virAsprintf fail. 4), change size_t for non-nagetive number in libxlDomainOpenConsole size_t i; size_t num = 0; 5), fix for make check (1), replace virAsprintf with VIR_STRDUP in two places (2), delete space. src/libxl/libxl_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_driver.c | 87 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4a0fba9..539d537 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -331,6 +331,90 @@ error: } static int +libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) +{ + const char *type = virDomainChrTypeToString(def->source.type); + int ret; + + if (!type) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected chr device type")); + return -1; + } + + switch (def->source.type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + if (VIR_STRDUP(*buf, type) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + if (virAsprintf(buf, "%s:%s", type, + def->source.data.file.path) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_TYPE_DEV: + if (VIR_STRDUP(*buf, def->source.data.file.path) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_TYPE_UDP: { + const char *connectHost = def->source.data.udp.connectHost; + const char *bindHost = def->source.data.udp.bindHost; + const char *bindService = def->source.data.udp.bindService; + + if (connectHost == NULL) + connectHost = ""; + if (bindHost == NULL) + bindHost = ""; + if (bindService == NULL) + bindService = "0"; + + ret = virAsprintf(buf, "udp:%s:%s@%s:%s", + connectHost, + def->source.data.udp.connectService, + bindHost, + bindService); + if (ret < 0) + return -1; + break; + + } + case VIR_DOMAIN_CHR_TYPE_TCP: + if (def->source.data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) + ret = virAsprintf(buf, "telnet:%s:%s%s", + def->source.data.tcp.host, + def->source.data.tcp.service, + def->source.data.tcp.listen ? ",server,nowait" : ""); + else + ret = virAsprintf(buf, "tcp:%s:%s%s", + def->source.data.tcp.host, + def->source.data.tcp.service, + def->source.data.tcp.listen ? ",server,nowait" : ""); + + if (ret < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: + ret = virAsprintf(buf, "unix:%s%s", + def->source.data.nix.path, + def->source.data.nix.listen ? ",server,nowait" : ""); + if (ret < 0) + return -1; + break; + + } + + return 0; +} + +static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) { libxl_domain_build_info *b_info = &d_config->b_info; @@ -403,6 +487,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0) goto error; + if (def->nserials) { + if (def->nserials > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Only one serial is supported by libxl")); + goto error; + } + if (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0) + goto error; + } + + if (def->nparallels) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Parallel is not supported")); + goto error; + } + /* * The following comment and calculation were taken directly from * libxenlight's internal function libxl_get_required_shadow_memory(): diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 2b4a281..861d689 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -34,6 +34,7 @@ # include "configmake.h" # include "virportallocator.h" # include "virobject.h" +# include "virchrdev.h" # define LIBXL_VNC_PORT_MIN 5900 @@ -94,6 +95,8 @@ struct _libxlDomainObjPrivate { /* per domain libxl ctx */ libxl_ctx *ctx; + /* console */ + virChrdevsPtr devs; libxl_evgen_domain_death *deathW; /* list of libxl timeout registrations */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 358d387..b3a8d50 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -417,6 +417,9 @@ libxlDomainObjPrivateAlloc(void) libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv); + if (!(priv->devs = virChrdevAlloc())) + return NULL; + return priv; } @@ -429,6 +432,7 @@ libxlDomainObjPrivateDispose(void *obj) libxl_evdisable_domain_death(priv->ctx, priv->deathW); libxl_ctx_free(priv->ctx); + virChrdevFree(priv->devs); } static void @@ -4493,6 +4497,88 @@ cleanup: return ret; } + +static int +libxlDomainOpenConsole(virDomainPtr dom, + const char *dev_name, + virStreamPtr st, + unsigned int flags) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + virDomainChrDefPtr chr = NULL; + libxlDomainObjPrivatePtr priv; + char *console = NULL; + + virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | + VIR_DOMAIN_CONSOLE_FORCE, -1); + + libxlDriverLock(driver); + vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + libxlDriverUnlock(driver); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (vm->def->nserials) + chr = vm->def->serials[0]; + + if (!chr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find character device %s"), + NULLSTR(dev_name)); + goto cleanup; + } + + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("character device %s is not using a PTY"), + NULLSTR(dev_name)); + goto cleanup; + } + + ret = libxl_primary_console_get_tty(priv->ctx, vm->def->id, &console); + if (ret) + goto cleanup; + + if (VIR_STRDUP(chr->source.data.file.path, console) < 0) + goto cleanup; + + /* handle mutually exclusive access to console devices */ + ret = virChrdevOpen(priv->devs, + &chr->source, + st, + (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0); + + if (ret == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Active console session exists for this domain")); + ret = -1; + } + +cleanup: + VIR_FREE(console); + if (vm) + virObjectUnlock(vm); + return ret; +} + static int libxlDomainSetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams) @@ -4875,6 +4961,7 @@ static virDriver libxlDriver = { .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */ .domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.2 */ .domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.2 */ + .domainOpenConsole = libxlDomainOpenConsole, /* 1.1.1 */ .domainIsActive = libxlDomainIsActive, /* 0.9.0 */ .domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */ .domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */ -- 1.8.1.4

3 3

[libvirt] [PATCH] Add logic for handling systemd-machined non-existance
by Daniel P. Berrange 24 Jul '13

24 Jul '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> If systemd machine does not exist, return -2 instead of -1, so that applications don't need to repeat the tedious error checking code Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/util/virsystemd.c | 13 ++++++++++- tests/virsystemdmock.c | 14 +++++++---- tests/virsystemdtest.c | 63 +++++++++++++++++++++++++++++++++++--------------- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8477cd3..11d1153 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -26,6 +26,7 @@ #include "virstring.h" #include "viralloc.h" #include "virutil.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_SYSTEMD @@ -38,6 +39,8 @@ * @rootdir: root directory of machine filesystem * @pidleader: PID of the leader process * @slice: name of the slice to place the machine in + * + * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not available */ int virSystemdCreateMachine(const char *name, const char *drivername, @@ -117,6 +120,7 @@ int virSystemdCreateMachine(const char *name, * allow further API calls to be made against the object. */ + VIR_DEBUG("Attempting to create machine via systemd"); if (virDBusCallMethod(conn, NULL, "org.freedesktop.machine1", @@ -135,8 +139,15 @@ int virSystemdCreateMachine(const char *name, (unsigned int)pidleader, rootdir ? rootdir : "", 1, "Slice", "s", - slicename) < 0) + slicename) < 0) { + virErrorPtr err = virGetLastError(); + if (err->code == VIR_ERR_DBUS_SERVICE && + STREQ(err->str2, "org.freedesktop.DBus.Error.ServiceUnknown")) { + virResetLastError(); + ret = -2; + } goto cleanup; + } ret = 0; diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c index 5f9cce6..1f4413c 100644 --- a/tests/virsystemdmock.c +++ b/tests/virsystemdmock.c @@ -60,16 +60,20 @@ dbus_bool_t dbus_connection_set_watch_functions(DBusConnection *connection ATTRI DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, int timeout_milliseconds ATTRIBUTE_UNUSED, - DBusError *error ATTRIBUTE_UNUSED) + DBusError *error) { - DBusMessage *reply; + DBusMessage *reply = NULL; dbus_message_set_serial(message, 7); - if (getenv("FAIL_NO_SERVICE")) + if (getenv("FAIL_BAD_SERVICE")) reply = dbus_message_new_error(message, - "org.freedesktop.DBus.Error.ServiceUnknown", - "The name org.freedesktop.machine1 was not provided by any .service files"); + "org.freedesktop.systemd.badthing", + "Something went wrong creating the machine"); + else if (getenv("FAIL_NO_SERVICE")) + dbus_set_error(error, + "org.freedesktop.DBus.Error.ServiceUnknown", + "%s", "The name org.freedesktop.machine1 was not provided by any .service files"); else reply = dbus_message_new_method_return(message); diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 3992722..bcf3ad3 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -82,35 +82,60 @@ static int testCreateNoSystemd(const void *opaque ATTRIBUTE_UNUSED) 3, 3, 3, 3, 4, 4, 4, 4 }; + int rv; setenv("FAIL_NO_SERVICE", "1", 1); - if (virSystemdCreateMachine("demo", - "qemu", - true, - uuid, - NULL, - 123, - false, - NULL) == 0) { + if ((rv = virSystemdCreateMachine("demo", + "qemu", + true, + uuid, + NULL, + 123, + false, + NULL)) == 0) { fprintf(stderr, "%s", "Unexpected create machine success\n"); return -1; } - virErrorPtr err = virGetLastError(); + if (rv != -2) { + fprintf(stderr, "%s", "Unexpected create machine error\n"); + return -1; + } + + return 0; +} - if (!err) { - fprintf(stderr, "No error raised"); +static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED) +{ + unsigned char uuid[VIR_UUID_BUFLEN] = { + 1, 1, 1, 1, + 2, 2, 2, 2, + 3, 3, 3, 3, + 4, 4, 4, 4 + }; + int rv; + + setenv("FAIL_BAD_SERVICE", "1", 1); + + if ((rv = virSystemdCreateMachine("demo", + "qemu", + true, + uuid, + NULL, + 123, + false, + NULL)) == 0) { + fprintf(stderr, "%s", "Unexpected create machine success\n"); return -1; } - if (err->code == VIR_ERR_DBUS_SERVICE && - STREQ(err->str2, "org.freedesktop.DBus.Error.ServiceUnknown")) - return 0; + if (rv != -1) { + fprintf(stderr, "%s", "Unexpected create machine error\n"); + return -1; + } - fprintf(stderr, "Unexpected error code %d / message %s\n", - err->code, err->str2); - return -1; + return 0; } static int @@ -122,7 +147,9 @@ mymain(void) ret = -1; if (virtTestRun("Test create machine ", 1, testCreateMachine, NULL) < 0) ret = -1; - if (virtTestRun("Test create nosystemd ", 1, testCreateNoSystemd, NULL) < 0) + if (virtTestRun("Test create no systemd ", 1, testCreateNoSystemd, NULL) < 0) + ret = -1; + if (virtTestRun("Test create bad systemd ", 1, testCreateBadSystemd, NULL) < 0) ret = -1; return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 1.8.1.4

2 1