- Devel - libvirt List Archives

[libvirt] [PATCH] docs: formatdomain: Document "spice" as a valid value for <graphics type=..>
by Richard W.M. Jones 26 Feb '16

26 Feb '16

Trivial documentation fix. Signed-off-by: Richard W.M. Jones <rjones(a)redhat.com> --- docs/formatdomain.html.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a524c17..5016772 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4891,7 +4891,8 @@ qemu-kvm -net nic,model=? /dev/null <dl> <dt><code>graphics</code></dt> <dd>The <code>graphics</code> element has a mandatory <code>type</code> - attribute which takes the value "sdl", "vnc", "rdp" or "desktop": + attribute which takes the value "sdl", "vnc", "spice", "rdp" or + "desktop": <dl> <dt><code>"sdl"</code></dt> <dd> -- 2.5.0

2 3

[libvirt] [PATCHv2 0/5] Fix machine type parsing regressions
by Ján Tomko 25 Feb '16

25 Feb '16

Patch 2/5 is the important one, fixing persistent non-QEMU domains. Ján Tomko (5): testCompareDomXML2XMLFiles: add parseFlags parameter Revert "Error out on missing machine type in machine configs" tests: add a test for persistent LXC XML parsing tests: add parseFlags to qemuxml2argvtest qemu: error out on missing machine type in configs src/conf/domain_conf.c | 6 --- src/qemu/qemu_domain.c | 6 +++ tests/bhyvexml2xmltest.c | 2 +- tests/genericxml2xmltest.c | 2 +- tests/lxcxml2xmltest.c | 14 ++++--- .../qemuxml2argv-missing-machine.xml | 30 +++++++++++++++ tests/qemuxml2argvtest.c | 43 ++++++++++++++-------- tests/qemuxml2xmltest.c | 4 +- tests/testutils.c | 5 ++- tests/testutils.h | 3 +- 10 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-missing-machine.xml -- 2.4.10

3 7

[libvirt] [PATCH] virfile: properly detect NFS storage
by Pavel Hrdina 25 Feb '16

25 Feb '16

Commit 35847860 introduced virFileUnlink() to fix an issue with deleting volumes on NFS root-squashed environment. This patch replace the uid and gid magic by virFileIsSharedFSType() to correctly detect that the volume is on NFS storage. This fixes the referenced bug. To reproduce this bug follow those steps on a domain with local storage: virsh start $domain virsh pool-refresh $pool virsh destroy $domain virsh vol-delete $volume $pool The thing is, that the pool-refresh will store qemu:qemu as uid:gid for that volume and after destroy the volume is relabeled back to root:root. Then you run vol-delete and the virFileRemove() function will try to unlink the file as a qemu:qemu process based on the uid and gid magic. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260356 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/util/virfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index f45e18f..f9c5bb1 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2334,13 +2334,16 @@ virFileRemove(const char *path, int status = 0, ret = 0; gid_t *groups; int ngroups; + int rc; /* If not running as root or if a non explicit uid/gid was being used for * the file/volume or the explicit uid/gid matches, then use unlink directly */ - if ((geteuid() != 0) || - ((uid == (uid_t) -1) && (gid == (gid_t) -1)) || - (uid == geteuid() && gid == getegid())) { + rc = virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS); + if (rc < 0) + return -EINVAL; + + if (rc == 0) { if (virFileIsDir(path)) return rmdir(path); else -- 2.7.1

3 3

[libvirt] [PATCH] libxl: unref objects in error paths
by Jim Fehlig 25 Feb '16

25 Feb '16

libxlMakeNic opens a virConnect object and takes a reference on a virNetwork object, but doesn't drop the references on all error paths. Rework the function to follow the standard libvirt pattern of using a local 'ret' variable to hold the function return value, performing all cleanup and returning 'ret' at a 'cleanup' label. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- I noticed these leaks while trying to rework the code to use a common virConnect object. That task has turned into quite an effort [1], but in the meantime the leaks in the current code should be plugged. [1] https://www.redhat.com/archives/libvir-list/2016-February/msg01188.html src/libxl/libxl_conf.c | 57 ++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b20df29..763f4c5 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1286,7 +1286,11 @@ libxlMakeNic(virDomainDefPtr def, { bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM; virDomainNetType actual_type = virDomainNetGetActualType(l_nic); + char *brname = NULL; + virNetworkPtr network = NULL; + virConnectPtr conn = NULL; virNetDevBandwidthPtr actual_bw; + int ret = -1; /* TODO: Where is mtu stored? * @@ -1312,64 +1316,50 @@ libxlMakeNic(virDomainDefPtr def, if (l_nic->model) { if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) - return -1; + goto cleanup; if (STREQ(l_nic->model, "netfront")) x_nic->nictype = LIBXL_NIC_TYPE_VIF; } if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0) - return -1; + goto cleanup; switch (actual_type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: if (VIR_STRDUP(x_nic->bridge, virDomainNetGetActualBridgeName(l_nic)) < 0) - return -1; + goto cleanup; /* fallthrough */ case VIR_DOMAIN_NET_TYPE_ETHERNET: if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) - return -1; + goto cleanup; if (l_nic->nips > 0) { x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address); if (!x_nic->ip) - return -1; + goto cleanup; } break; case VIR_DOMAIN_NET_TYPE_NETWORK: { - bool fail = false; - char *brname = NULL; - virNetworkPtr network; - virConnectPtr conn; - if (!(conn = virConnectOpen("xen:///system"))) - return -1; + goto cleanup; if (!(network = virNetworkLookupByName(conn, l_nic->data.network.name))) { - virObjectUnref(conn); - return -1; + goto cleanup; } if (l_nic->nips > 0) { x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address); if (!x_nic->ip) - return -1; + goto cleanup; } - if ((brname = virNetworkGetBridgeName(network))) { - if (VIR_STRDUP(x_nic->bridge, brname) < 0) - fail = true; - } else { - fail = true; - } - - VIR_FREE(brname); + if (!(brname = virNetworkGetBridgeName(network))) + goto cleanup; - virObjectUnref(network); - virObjectUnref(conn); - if (fail) - return -1; + if (VIR_STRDUP(x_nic->bridge, brname) < 0) + goto cleanup; break; } case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -1385,18 +1375,18 @@ libxlMakeNic(virDomainDefPtr def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported interface type %s"), virDomainNetTypeToString(l_nic->type)); - return -1; + goto cleanup; } if (l_nic->domain_name) { #ifdef LIBXL_HAVE_DEVICE_BACKEND_DOMNAME if (VIR_STRDUP(x_nic->backend_domname, l_nic->domain_name) < 0) - return -1; + goto cleanup; #else virReportError(VIR_ERR_XML_DETAIL, "%s", _("this version of libxenlight does not " "support backend domain name")); - return -1; + goto cleanup; #endif } @@ -1438,7 +1428,14 @@ libxlMakeNic(virDomainDefPtr def, x_nic->rate_interval_usecs = 50000UL; } - return 0; + ret = 0; + + cleanup: + VIR_FREE(brname); + virObjectUnref(network); + virObjectUnref(conn); + + return ret; } static int -- 2.6.1

3 3

[libvirt] [PATCH v3 0/3] Add capability for text based polkit authentication for virsh
by John Ferlan 25 Feb '16

25 Feb '16

v2: http://www.redhat.com/archives/libvir-list/2016-February/msg00618.html Adjustments since v2: Patch 1 - Change not only the message (as requested), but also use a different and new error code (VIR_ERR_AUTH_UNAVAILABLE). Patch 2 - Added a check for "if (!isatty(STDIN_FILENO))" Used a new parameter for pkttyagent call - "--notify-fd", which is documented as "To get notified when the authentication agent has been registered either listen to the Changed D-Bus signal or use --notify-fd to pass the number of a file descriptor that has been passed to the program. This file descriptor will then be closed when the authentication agent has been successfully registered. Followed the systemd mechanism. If it's felt that a timeout of -1 is too dangerous, I'd be fine with changing it. Patch 3 - Since we now can determine our failure based on err->code, use the new VIR_ERR_AUTH_UNAVAILABLE to attempt the AgentCreate. Also, since virPolkitAgentCreate now will wait for the agent to start before returning, the agentstart counter is removed. John Ferlan (3): polkit: Adjust message when authentication agent isn't found util: Introduce API's for Polkit text authentication virsh: Add support for text based polkit authentication include/libvirt/virterror.h | 3 +- src/libvirt_private.syms | 2 ++ src/util/virerror.c | 8 ++++- src/util/virpolkit.c | 85 ++++++++++++++++++++++++++++++++++++++++++--- src/util/virpolkit.h | 5 +++ tests/virpolkittest.c | 8 +++-- tools/virsh.c | 38 +++++++++++++++++--- tools/virsh.h | 2 ++ 8 files changed, 138 insertions(+), 13 deletions(-) -- 2.5.0

3 15

[libvirt] [PATCH 1/2] netdev: Use virNetDevIsVirtualFunction() properly
by Andrea Bolognani 25 Feb '16

25 Feb '16

virNetDevIsVirtualFunction() returns 1 if the interface is a virtual function, 0 if it isn't and -1 on error. This means that, despite the name suggesting otherwise, using it as a predicate is not correct. Fix two callers that were doing so adding an explicit check on the return value. --- src/util/virnetdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index ea95552..9e39c26 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2531,7 +2531,7 @@ virNetDevReplaceNetConfig(const char *linkdev, int vf, int ret = -1; char *pfdevname = NULL; - if (vf == -1 && virNetDevIsVirtualFunction(linkdev)) { + if (vf == -1 && virNetDevIsVirtualFunction(linkdev) == 1) { /* If this really *is* a VF and the caller just didn't know * it, we should set the MAC address via PF+vf# instead of * setting directly via VF, because the latter will be @@ -2571,7 +2571,7 @@ virNetDevRestoreNetConfig(const char *linkdev, int vf, const char *stateDir) char *pfdevname = NULL; const char *vfdevname = NULL; - if (vf == -1 && virNetDevIsVirtualFunction(linkdev)) { + if (vf == -1 && virNetDevIsVirtualFunction(linkdev) == 1) { /* If this really *is* a VF and the caller just didn't know * it, we should set the MAC address via PF+vf# instead of * setting directly via VF, because the latter will be -- 2.5.0

2 2

[libvirt] [PATCH 2/2] hostdev: Remove temporary variable when checking for VF
by Andrea Bolognani 25 Feb '16

25 Feb '16

The virHostdevIsVirtualFunction() was called exactly twice, and in both cases the return value was saved to a temporary variable before being checked. This would be okay if it improved readability, but in this case is pretty pointless. Get rid of the temporary variable and check the return value directly; while at it, change the check from '<= 0' to '!= 1' to align it with the way other similar *IsVirtualFunction() functions are used thorough the code. --- src/util/virhostdev.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9ae217d..2db0da0 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -416,10 +416,8 @@ virHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev, int vf = -1; int vlanid = -1; bool port_profile_associate = true; - int isvf; - isvf = virHostdevIsVirtualFunction(hostdev); - if (isvf <= 0) { + if (virHostdevIsVirtualFunction(hostdev) != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Interface type hostdev is currently supported on" " SR-IOV Virtual Functions only")); @@ -489,7 +487,6 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, int ret = -1; int vf = -1; bool port_profile_associate = false; - int isvf; /* This is only needed for PCI devices that have been defined * using <interface type='hostdev'>. For all others, it is a NOP. @@ -497,8 +494,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev, if (!virHostdevIsPCINetDevice(hostdev)) return 0; - isvf = virHostdevIsVirtualFunction(hostdev); - if (isvf <= 0) { + if (virHostdevIsVirtualFunction(hostdev) != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Interface type hostdev is currently supported on" " SR-IOV Virtual Functions only")); -- 2.5.0

2 2

[libvirt] [python PATCH 0/4] formatting and (un)signed improvements
by Pavel Hrdina 25 Feb '16

25 Feb '16

Pavel Hrdina (4): libvirt-override: all flags should be defined as unsigned int libvirt-override: fix PyArg_ParseTuple for unsigned int libvirt-override: fix PyArg_ParseTuple for unsigned long long libvirt-override: fix PyArg_ParseTuple for size_t libvirt-override.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) -- 2.7.1

2 5

[libvirt] [PATCH 0/3] More Coverity changes
by John Ferlan 25 Feb '16

25 Feb '16

All the issues here are found by a later version of Coverity than the one used by our internal testing. Two were false positives and one just an ordering issue with the virCheckFlags in the zfs backend. The future coverity really has a problem with strtok_r usage after a VIR_STRDUP which only checks < 0 because it's perfectly fine for the "source" to be NULL and a < 0 check won't fail. The reason why this is a problem is the target is then NULL and used for the strtok_r call. Code inspection finds the cases we have are all false positives; however, rather than making the <= or sa_assert() type check to validate that - I chose to use the virStringSplitCount in order to split and inspect the input string. It was much cleaner at least on the inspection front. The storage changes related to checking (ret == -1) resulted in a RESOURCE_LEAK false positive. As it turns out, the only reason why -1 was being check was to determine whether or not the VIR_APPEND_ELEMENT insertion failed. If that succeeds, then 'vol' is cleared anyway, so truly we only need to attempt the free if we have a new vol. The virStorageVolDefFree will quietly return if vol == NULL. John Ferlan (3): openvz: Use virStringSplitCount instead of strtok_r zfs: Resolve RESOURCE_LEAK storage: No need to check ret after VIR_APPEND_ELEMENT src/openvz/openvz_conf.c | 32 +++++++++++--------------------- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_zfs.c | 6 ++++-- 3 files changed, 16 insertions(+), 24 deletions(-) -- 2.5.0

2 5

[libvirt] [PATCH v4 0/2] notify about reverting to a snapshot
by Dmitry Andreev 25 Feb '16

25 Feb '16

Reverting to a snapshot may change domain configuration but there will be no events about that in some cases. Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148 This patch-set introduces new event and emits it in qemuDomainRevertToSnapshot. v4: rebase v3: [2/2] event is emited only in the case when stopped domain is reverted to a stopped domain. Dmitry Andreev (2): Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event qemu: emit 'defined' event after reverted to snapshot without state changes examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 8 +++++++- tools/virsh-domain.c | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-) -- 1.8.3.1

2 3