- Devel - libvirt List Archives

[libvirt] [PATCH] virschematest: Access the right directory containing XMLs
by Michal Privoznik 08 Jun '16

08 Jun '16

So the story goes like this. The testSchemaDirs() function is called with: a) the schema file, b) list of the directories that contains XMLs documents that should be checked against the schema file from a). However, the directories in the list are really just their names and it's up to testSchemaDirs to construct the absolute path and call testSchemaDir() which then does the actual validation. The absolute path is constructed, but never actually used (maybe due to a typo). Thus a VPATH build is broken. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/virschematest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index f4d41bd..c173037 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -140,7 +140,7 @@ testSchemaDirs(const char *schema, ...) ret = -1; goto cleanup; } - if (testSchemaDir(schema, validator, dir) < 0) + if (testSchemaDir(schema, validator, dir_path) < 0) ret = -1; VIR_FREE(dir_path); } -- 2.8.3

2 1

[libvirt] [PATCH] virsh: domdisplay: if listen is 0.0.0.0 or [::] print address from URI
by Pavel Hrdina 08 Jun '16

08 Jun '16

Currently if a guest has listen address 0.0.0.0 or [::] and you run "virsh domdisplay $domain" you always get "spice://localhost:$port". We want to print better address if someone is connected from a different computer using "virsh -c qemu+ssh://some.host/system". This patch fixes the behavior of virsh to print in this case "spice://some.host:$port". Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1332446 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- tools/virsh-domain.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8d7ff61..93c7050 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -57,6 +57,7 @@ #include "virtypedparam.h" #include "virxml.h" #include "virsh-nodedev.h" +#include "viruri.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -10617,6 +10618,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) VIR_FREE(xpath); } + /* If listen_addr is 0.0.0.0 or [::] we should try to parse URI and set + * listen_addr based on current URI. */ + if (listen_addr) { + if (virSocketAddrParse(&addr, listen_addr, AF_UNSPEC) > 0 && + virSocketAddrIsWildcard(&addr)) { + + virURIPtr uri = virURIParse(ctl->connname); + + /* It's safe to free the listen_addr even if parsing of URI + * fails, if there is no listen_addr we will print "localhost". */ + VIR_FREE(listen_addr); + + if (uri && VIR_STRDUP(listen_addr, uri->server) < 0) + goto cleanup; + } + } + /* We can query this info for all the graphics types since we'll * get nothing for the unsupported ones (just rdp for now). * Also the parameter '--include-password' was already taken @@ -10638,9 +10656,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) virBufferAsprintf(&buf, ":%s@", passwd); /* Then host name or IP */ - if (!listen_addr || - (virSocketAddrParse(&addr, listen_addr, AF_UNSPEC) > 0 && - virSocketAddrIsWildcard(&addr))) + if (!listen_addr) virBufferAddLit(&buf, "localhost"); else if (strchr(listen_addr, ':')) virBufferAsprintf(&buf, "[%s]", listen_addr); -- 2.8.3

2 1

[libvirt] [PATCH] qemu_process: print generic error if qemu exit without printing any error
by Pavel Hrdina 08 Jun '16

08 Jun '16

In this case we would print only the libvirt part of error message without any explanation what happened: "error: internal error: process exited while connecting to monitor:" Let's print a generic error if this happens. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1335617 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/qemu/qemu_process.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e847cd1..86701da 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1808,6 +1808,13 @@ qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt, if (qemuProcessReadLog(logCtxt, &logmsg) < 0) return -1; + if (virStringIsEmpty(logmsg)) { + VIR_FREE(logmsg); + if (VIR_STRDUP(logmsg, _("qemu process exited without any " + "error printed out")) < 0) + return -1; + } + virResetLastError(); virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: %s"), msgprefix, logmsg); -- 2.8.3

2 1

[libvirt] [PATCH RFC 00/16] Add support for LUKS encrypted devices
by John Ferlan 08 Jun '16

08 Jun '16

Patches 1-3 were posted separately: http://www.redhat.com/archives/libvir-list/2016-June/msg00256.html But perhaps seeing the final direction will make things more clear as to why a "real" flag system wasn't used and keeping the current paradigm of constant value returns still works just fine. Patches 4-5 were posted separately: http://www.redhat.com/archives/libvir-list/2016-June/msg00091.html (4) http://www.redhat.com/archives/libvir-list/2016-June/msg00094.html (5) Although at one point patch 4 had an ACK: http://www.redhat.com/archives/libvir-list/2016-May/msg02115.html It wasn't clear if the more recent review rescinded that, so it still remains "in the list". I understand the concern about adding secret to cfg.mk checking, but without a better idea of how to handle - I left things as they were. Patches 6-16 are all new. Some parts are separable, but rather than continue piecemeal I just figured going with an RFC will at least Patch 6 is only there to "prove" that using the current encryption paradigm XML still works, although if I've read the tea leaves correctly, the qemu support isn't working as desired/expected. Patch 7 adds "usage" as an XML attribute for encryption and the associated tests with that. I've chosen to "reuse" the <encryption> XML element rather than inventing something new. I'm not opposed to something new, but let's decide up a name quickly... Patch 8-9 adds the ability for the storage backend to create/recognize a luks volume Patches 10-13 adds support for luks encryption in the storage backend. The new "<secret>" format uses "luks" as the usage type and "<key>" as the 'name'. If those names cause angst, I'm fine with changing, but just give a better suggestion! Adding <cipher> and <ivgen> were a result of using qemu constructs from qemu commit id '3e308f20'. Since we are parsing something new, I figure failing in the domain parse code for this new type was acceptible as opposed to some post processing check. Patches 14-16 adds support for luks encryption to the domain using <encryption type='luks'... <secret format='key' usage/uuid='xxx'>> I've tested using a "good" and "bad" password and got the expected results for starting a domain. I did not add 'virsh vol-create-as' support just yet. I figured that would be less to go back and redo if the names of elements changes. I've also run the changes through Coverity with no new issues detected. The whole series is a result of the following bz: https://bugzilla.redhat.com/show_bug.cgi?id=1301021 John Ferlan (16): storage: Adjust qemu-img switches check storage: Create helper to set backing for CreateQemuImg code storage: Create helper to set options for CreateQemuImg code storage: Use virSecretGetSecretString secret: Move virStorageSecretType to secret_util and rename tests: Adjust tests for encrypted storage util: Add 'usage' for encryption util: Modify the FileTypeInfo for meta data checks util: Add 'luks' to the FileTypeInfo conf: Add new secret type "luks" encryption: Add luks parsing for storageencryption encryption: Add <cipher> and <ivgen> to encryption storage: Add support to create a luks volume qemu: Change protocol parameter for secret setup qemu: Remove authdef from secret setup qemu: Add luks support for domain disk cfg.mk | 2 +- docs/aclpolkit.html.in | 4 + docs/formatsecret.html.in | 60 ++- docs/formatstorageencryption.html.in | 115 ++++- docs/schemas/secret.rng | 10 + docs/schemas/storagecommon.rng | 58 ++- include/libvirt/libvirt-secret.h | 3 +- src/Makefile.am | 1 + src/access/viraccessdriverpolkit.c | 13 + src/conf/domain_conf.c | 11 + src/conf/secret_conf.c | 26 +- src/conf/secret_conf.h | 3 +- src/conf/virsecretobj.c | 5 + src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 8 +- src/qemu/qemu_domain.c | 154 ++++--- src/qemu/qemu_process.c | 18 +- src/secret/secret_util.c | 18 +- src/secret/secret_util.h | 22 +- src/storage/storage_backend.c | 480 +++++++++++++++------ src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 10 +- src/storage/storage_backend_gluster.c | 2 + src/storage/storage_backend_iscsi.c | 55 +-- src/storage/storage_backend_rbd.c | 49 +-- src/util/virendian.h | 24 ++ src/util/virqemu.c | 23 + src/util/virqemu.h | 6 + src/util/virstorageencryption.c | 166 ++++++- src/util/virstorageencryption.h | 18 +- src/util/virstoragefile.c | 125 ++++-- src/util/virstoragefile.h | 18 +- tests/qemuargv2xmltest.c | 4 +- .../qemuxml2argv-encrypted-disk-usage.args | 24 ++ .../qemuxml2argv-encrypted-disk-usage.xml | 32 ++ .../qemuxml2argv-encrypted-disk.args | 26 +- .../qemuxml2argv-encrypted-disk.xml | 4 +- .../qemuxml2argv-luks-disk-cipher.args | 36 ++ .../qemuxml2argv-luks-disk-cipher.xml | 41 ++ .../qemuxml2argvdata/qemuxml2argv-luks-disks.args | 36 ++ tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 41 ++ tests/qemuxml2argvtest.c | 14 +- .../qemuxml2xmlout-encrypted-disk-usage.xml | 36 ++ .../qemuxml2xmlout-encrypted-disk.xml | 4 +- .../qemuxml2xmlout-luks-disk-cipher.xml | 45 ++ .../qemuxml2xmlout-luks-disks.xml | 45 ++ tests/qemuxml2xmltest.c | 3 + tests/secretxml2xmlin/usage-luks.xml | 7 + tests/secretxml2xmltest.c | 1 + tests/storagevolxml2argvdata/qcow2-flag.argv | 2 - .../qcow2-nobacking-convert-flag.argv | 2 - .../qcow2-nobacking-convert-none.argv | 2 - .../qcow2-nobacking-flag.argv | 1 - .../qcow2-nobacking-none.argv | 1 - tests/storagevolxml2argvdata/qcow2-none.argv | 1 - tests/storagevolxml2argvtest.c | 25 +- tests/storagevolxml2xmlin/vol-luks-cipher.xml | 23 + tests/storagevolxml2xmlin/vol-luks.xml | 21 + tests/storagevolxml2xmlout/vol-luks-cipher.xml | 23 + tests/storagevolxml2xmlout/vol-luks.xml | 21 + tests/storagevolxml2xmltest.c | 2 + tests/virendiantest.c | 18 + 63 files changed, 1619 insertions(+), 435 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk-usage.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk-usage.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-encrypted-disk-usage.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml create mode 100644 tests/secretxml2xmlin/usage-luks.xml delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv create mode 100644 tests/storagevolxml2xmlin/vol-luks-cipher.xml create mode 100644 tests/storagevolxml2xmlin/vol-luks.xml create mode 100644 tests/storagevolxml2xmlout/vol-luks-cipher.xml create mode 100644 tests/storagevolxml2xmlout/vol-luks.xml -- 2.5.5

2 19

[libvirt] [PATCH 0/3] qemu: fix startup policy checking
by Peter Krempa 08 Jun '16

08 Jun '16

One of the recent refactors broke disk startup policy checking. Fix it with a few cleanups. Peter Krempa (3): qemu: domain: Sanitize return value handling in disk presence checker qemu: process: Unexport qemuProcessStartValidate qemu: process: Call disk startup policy check after cloning domain def src/qemu/qemu_domain.c | 16 +++++----------- src/qemu/qemu_process.c | 14 +++++++++----- src/qemu/qemu_process.h | 7 ------- 3 files changed, 14 insertions(+), 23 deletions(-) -- 2.8.3

2 7

[libvirt] [PATCH] qemu: driver: Unset log file watcher after restoring a VM save file
by Peter Krempa 08 Jun '16

08 Jun '16

qemuProcessStart does not unset the infrastructure that retrieves errors from the qemu log file in case of migration. As this wasn't handled properly in qemuDomainSaveImageStartVM we kept the logging context/fd open for the lifetime of the VM rather than closing it after it's not needed. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325080 --- src/qemu/qemu_driver.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d3e3d..4f09630 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6509,6 +6509,11 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (!restored) goto cleanup; + /* qemuProcessStart doesn't unsed the qemu error reporting infrastructure + * in case of migration (which is used in this case) so we need to reset it + * so that the handle to virtlogd is not held open unnecessarily */ + qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL); + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); -- 2.8.3

2 2

[libvirt] [PATCH] xen: Also add sub-type for driver=tap2 in xen-xm
by Philipp Hahn 07 Jun '16

07 Jun '16

tap2 only handles 'aio', but not 'raw', which must be explicitly given: | $ virsh domxml-to-native yyy.xml > yyy.xm | $ xm new yyy.xm | Error: tap:/srv/xen/xxx.img not a valid disk type | $ sed -i -e 's/tap2:/&aio:/' yyy.xm | $ xm new yyy.xm Fix reading and writing "xen-xm" format for "tap2" by handling it the same as "tap". --- src/xenconfig/xen_xm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index e09d97e..6556886 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -196,7 +196,8 @@ xenParseXMDisk(virConfPtr conf, virDomainDefPtr def) } /* And the sub-type for tap:XXX: type */ - if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap")) { + if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap") || + STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap2")) { char *driverType; if (!(tmp = strchr(src, ':'))) @@ -298,7 +299,7 @@ xenFormatXMDisk(virConfValuePtr list, else type = virStorageFileFormatTypeToString(format); virBufferAsprintf(&buf, "%s:", driver); - if (STREQ(driver, "tap")) + if (STREQ(driver, "tap") || STREQ(driver, "tap2")) virBufferAsprintf(&buf, "%s:", type); } else { switch (virDomainDiskGetType(disk)) { -- 2.1.4

2 6

Re: [libvirt] Channel paths
by Sascha Silbe 07 Jun '16

07 Jun '16

Dear Daniel, [CC += libvir-list, didn't notice before this thread is on qemu-devel only] "Daniel P. Berrange" <berrange(a)redhat.com> writes: > On Thu, Jun 02, 2016 at 09:27:45PM +0200, Sascha Silbe wrote: >> Since 71408079 [qemu: Don't bother user with libvirt-internal paths], >> the path chosen by libvirt isn't included in the domain XML anymore. So >> now I need to choose the path inside the application. The only safe way >> to do that is by using something in an application-managed namespace >> (e.g. /var/lib/myapp/foo or /tmp/myapp/foo); I certainly wouldn't want >> to second guess what paths would be safe inside the libvirt namespace >> (/var/lib/libvirt/qemu). Except now I hear that anything outside >> /var/lib/libvirt/qemu is not guaranteed to work due to e.g. the SELinux >> policy configured by libvirt... > > IIUC that change 71408079 ought to only apply to the persistent XML > configuration on disk. When the guest is running the live XML ought > to still report the path libvirt chose, so you should still be able > to see it while running. If that isn't the case then please raise a > bug, because we certainly expect apps to be able to discover the > path libvirt picked for exactly the reason you describe OK, this makes a lot more sense now (persistent vs. live). Just tried it with current libvirt master and it works, thanks. It even worked with 71408079 itself. I'm pretty sure things were broken some time in between, but a few quick probes didn't turn up a faulty version and I'm not curious enough to do a linear search. Cannot completely rule out other effects having played a role, either. Sascha -- Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg https://se-silbe.de/ USt-IdNr. DE281696641

1 0

[libvirt] [PATCH v3 0/4] Replace VIR_ERROR with standard vir*Error in state driver init
by Jovanka Gulicoska 07 Jun '16

07 Jun '16

Replace VIR_ERROR logging macros for error reporting in driver startup routines with vir*Error function. Don't use virStrerror and remove ebuf. Link to task: http://wiki.libvirt.org/page/BiteSizedTasks#Replace_VIR_ERROR_with_standard… Jovanka Gulicoska (4): uml: Replace VIR_ERROR with standard vir*Error in state driver init qemu: Replace VIR_ERROR with standard vir*Error in state driver init xen: Replace VIR_ERROR with standard vir*Error in state driver init node_device: Replace VIR_ERROR with standard vir*Error in state driver init src/node_device/node_device_hal.c | 26 +++++++++++------- src/node_device/node_device_udev.c | 49 ++++++++++++++++++++-------------- src/qemu/qemu_driver.c | 54 +++++++++++++++++++++++--------------- src/uml/uml_driver.c | 20 ++++++-------- src/xen/xen_driver.c | 5 ++-- 5 files changed, 90 insertions(+), 64 deletions(-) -- 2.5.5

2 5

[libvirt] [PATCH] Updated to deal with specifying user IDs to that do not map to usernames
by Roy Keene 07 Jun '16

07 Jun '16

Patch to libvirt master to avoid failing when a user ID is specified, e.g. for <seclabel type='dac'>, that does not map to a user name. This is useful if you want to run each VM as a separate user and not bother creating an /etc/passwd entry for each UID. It compiles but is as yet untested. --- src/util/virutil.c | 69 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index d80d994..ae95237 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -790,26 +790,57 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir) if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) goto cleanup; } - if (rc != 0) { - virReportSystemError(rc, - _("Failed to find user record for uid '%u'"), - (unsigned int) uid); - goto cleanup; - } else if (pw == NULL) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Failed to find user record for uid '%u'"), - (unsigned int) uid); - goto cleanup; - } - if (name && VIR_STRDUP(*name, pw->pw_name) < 0) - goto cleanup; - if (group) - *group = pw->pw_gid; - if (dir && VIR_STRDUP(*dir, pw->pw_dir) < 0) { - if (name) - VIR_FREE(*name); - goto cleanup; + if (rc != 0 || pw == NULL) { + /* + * If the user does not exist or its data is not present, return + * a created username. + */ + VIR_FREE(strbuf); + + strbuflen = 128; + + if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { + return(-1); + } + + /* + * Fake user home directory: / + */ + if (dir) { + if (VIR_STRDUP(*dir, "/") < 0) { + goto cleanup; + } + } + + /* + * Fake user GID: Same as UID + */ + if (group) { + *group = (gid_t) uid; + } + + /* + * Fake user name: Same as UID (in string) + */ + snprintf(strbuf, strbuflen, "%llu", (unsigned long long) uid); + + if (name && VIR_STRDUP(*name, strbuf) < 0) { + if (dir) { + VIR_FREE(*dir); + } + goto cleanup; + } + } else { + if (name && VIR_STRDUP(*name, pw->pw_name) < 0) + goto cleanup; + if (group) + *group = pw->pw_gid; + if (dir && VIR_STRDUP(*dir, pw->pw_dir) < 0) { + if (name) + VIR_FREE(*name); + goto cleanup; + } } ret = 0; -- 2.7.4

4 8