August 2015 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Report better error message when renaming to existing domain name
by Martin Kletzander 19 Aug '15

19 Aug '15

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_driver.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 99a3817ff5ab..16061a51d4a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19892,6 +19892,7 @@ static int qemuDomainRename(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virQEMUDriverConfigPtr cfg = NULL; virDomainObjPtr vm = NULL; + virDomainObjPtr tmp_dom = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -19946,6 +19947,21 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } + /* + * This is a rather racy check, but still better than reporting + * internal error. And since new_name != name here, there's no + * deadlock imminent. + */ + tmp_dom = virDomainObjListFindByName(driver->domains, new_name); + if (tmp_dom) { + virObjectUnlock(tmp_dom); + virObjectUnref(tmp_dom); + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain with name '%s' already exists"), + new_name); + goto endjob; + } + if (VIR_STRDUP(new_dom_name, new_name) < 0) goto endjob; -- 2.5.0

2 1

[libvirt] [PATCH] qemu: fix the error cover issue in qemuDomainAddCgroupForThread
by Luyao Huang 18 Aug '15

18 Aug '15

Just like commit 704cf06, the error already will be set in virCgroup* function, and virCgroupAddTask will return -1, so We will always report error "Operation not permitted" in this place. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- src/qemu/qemu_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa655b5..e0d7fa5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4606,9 +4606,6 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, /* Add pid/thread to the cgroup */ rv = virCgroupAddTask(new_cgroup, pid); if (rv < 0) { - virReportSystemError(-rv, - _("unable to add id %d task %d to cgroup"), - idx, pid); virCgroupRemove(new_cgroup); goto error; } -- 1.8.3.1

4 4

[libvirt] [PATCH] qemu: fix not update weight in def after success
by Luyao Huang 18 Aug '15

18 Aug '15

https://bugzilla.redhat.com/show_bug.cgi?id=1253107 Update the weight in vm def to fix this. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- src/qemu/qemu_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a984a8..0b984dc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9170,6 +9170,8 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { if (virCgroupSetBlkioWeight(priv->cgroup, param->value.ui) < 0) ret = -1; + else + def->blkio.weight = param->value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || -- 1.8.3.1

3 2

[libvirt] [PATCH] virsh: fix output the incorrect error after try failed
by Luyao Huang 18 Aug '15

18 Aug '15

https://bugzilla.redhat.com/show_bug.cgi?id=1254152 When we use some virsh cmd which need specify domain name/id/uuid, if the command get failure we will get error like this: # virsh domif-setlink 123 vnet1 up error: interface (target: vnet1) not found error: Domain not found: no domain with matching id 123 The second line should be reset after call virshLookupDomainInternal, because after some tries we get domain pointer, so output error during we tried will make user confuse. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- tools/virsh-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 173bb15..69c5562 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -98,6 +98,8 @@ virshLookupDomainInternal(vshControl *ctl, dom = virDomainLookupByName(priv->conn, name); } + vshResetLibvirtError(); + if (!dom) vshError(ctl, _("failed to get domain '%s'"), name); -- 1.8.3.1

3 2

[libvirt] [PATCH] virconf: fix the inconsistent name
by Cao jin 18 Aug '15

18 Aug '15

Fix the name inconsistency between func comments and parameter of virConfGetValue/virConfSetValue Signed-off-by: Cao jin <caoj.fnst(a)cn.fujitsu.com> --- src/util/virconf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virconf.c b/src/util/virconf.c index 86d76b5..ab98c5c 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -840,7 +840,7 @@ virConfFree(virConfPtr conf) /** * virConfGetValue: * @conf: a configuration file handle - * @entry: the name of the entry + * @setting: the name of the entry * * Lookup the value associated to this entry in the configuration file * @@ -870,7 +870,7 @@ virConfGetValue(virConfPtr conf, const char *setting) /** * virConfSetValue: * @conf: a configuration file handle - * @entry: the name of the entry + * @setting: the name of the entry * @value: the new configuration value * * Set (or replace) the value associated to this entry in the configuration -- 2.1.0

2 2

[libvirt] [PATCH 0/5] Make sure internal blkiotune values are in sync
by Martin Kletzander 18 Aug '15

18 Aug '15

We were blindly setting blkiotune values for devices, but kernel can throw some of them away. This series reworks the logic the same wayother tuning values are updated. That is, after the value gets set, it is read back again to make sure internal structures are in sync and we can return the values from them. Little example should clear up everything. Before: ======= $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,18446744073709551614 $ virsh blkiotune dummy weight : 500 device_weight : device_read_iops_sec: device_write_iops_sec: device_read_bytes_sec: /dev/sda,18446744073709551614 device_write_bytes_sec: $ cat /sys/fs/cgroup/blkio/machine/dummy.libvirt-qemu/blkio.throttle.read_bps_device 8:0 18446744073709551614 $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,18446744073709551615 $ virsh blkiotune dummy weight : 500 device_weight : device_read_iops_sec: device_write_iops_sec: device_read_bytes_sec: /dev/sda,18446744073709551615 device_write_bytes_sec: $ cat /sys/fs/cgroup/blkio/machine/dummy.libvirt-qemu/blkio.throttle.read_bps_device $ After: ====== $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,18446744073709551614 $ virsh blkiotune dummy weight : 500 device_weight : device_read_iops_sec: device_write_iops_sec: device_read_bytes_sec: /dev/sda,18446744073709551614 device_write_bytes_sec: $ cat /sys/fs/cgroup/blkio/machine/dummy.libvirt-qemu/blkio.throttle.read_bps_device 8:0 18446744073709551614 $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,18446744073709551615 $ virsh blkiotune dummy weight : 500 device_weight : device_read_iops_sec: device_write_iops_sec: device_read_bytes_sec: device_write_bytes_sec: $ cat /sys/fs/cgroup/blkio/machine/dummy.libvirt-qemu/blkio.throttle.read_bps_device $ Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1165580 Martin Kletzander (5): util: Add virStringGetFirstWithPrefix util: Add virCgroupGetBlockDevString util: Add getters for cgroup block device I/O throttling lxc: Sync BlkioDevice values when setting them in cgroups qemu: Sync BlkioDevice values when setting them in cgroups src/libvirt_private.syms | 6 + src/lxc/lxc_cgroup.c | 20 ++- src/lxc/lxc_driver.c | 25 ++- src/qemu/qemu_cgroup.c | 20 ++- src/qemu/qemu_driver.c | 25 ++- src/util/vircgroup.c | 457 +++++++++++++++++++++++++++++++++++------------ src/util/vircgroup.h | 20 +++ src/util/virstring.c | 17 ++ src/util/virstring.h | 2 + 9 files changed, 459 insertions(+), 133 deletions(-) -- 2.5.0

2 8

[libvirt] [PATCH] qemuDomainRename: Don't leave a domain locked uppon fail
by Michal Privoznik 18 Aug '15

18 Aug '15

Well, yet again one case of 'goto cleanup' while 'goto endjob' was needed. Sorry. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed as trivial. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 180dd88..99a3817 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19943,7 +19943,7 @@ static int qemuDomainRename(virDomainPtr dom, if (STREQ(vm->def->name, new_name)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Can't rename domain to itself")); - goto cleanup; + goto endjob; } if (VIR_STRDUP(new_dom_name, new_name) < 0) -- 2.4.6

1 0

[libvirt] [PATCH 0/3] Some minor IOThread API adjustments/checks
by John Ferlan 18 Aug '15

18 Aug '15

As a result of the review of a related issue for virDomainAddIOThread it was noted that it's only our implementation for qemu that doesn't want an iothread_id == 0, see: http://www.redhat.com/archives/libvir-list/2015-August/msg00310.html Continued IRC chats with the submittor of that patch discovered some inconsistencies which are dealt with in this patch series... Patch 1 - Remove the iothread_id arg check in virDomainPinIOThread. Since 0 was allowed by the API anyway and left up to the hypervisor. The argument is unsigned - so in reality all that was being checked was 0 - other similar callers work on 'int' values not 'unsigned int'. Patch 2 - Adjust the description in virDomainAddIOThread to remove the "positive non-zero value" and then rework the text to be clearer that it's up to the hypervisor to decide what values are legal and how to handle possible duplication. Patch 3 - Add check in qemu_driver Add/Del IOThread API for the invalid 0 value for iothread_id NOTE: It's not necessary to add a specific illegal argument check for iothread_id = 0 in qemuDomainPinIOThread since both the live and config paths will first search for the iothread_id value and fail if not found in the iothreadid list. I'm not opposed to adding a specific error message if so desired... John Ferlan (3): api: Remove check on iothread_id arg in virDomainPinIOThread api: Adjust comment for virDomainAddIOThread qemu: Add check for invalid iothread_id in qemuDomainChgIOThread src/libvirt-domain.c | 7 +++---- src/qemu/qemu_driver.c | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) -- 2.1.0

2 4

[libvirt] [PATCH 0/2] 2 small fix for domrename
by Luyao Huang 17 Aug '15

17 Aug '15

*** BLURB HERE *** Luyao Huang (2): virsh: fix always return false in domrename libvirt-domain: forbid use virDomainRename in readonly connection src/libvirt-domain.c | 1 + tools/virsh-domain.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) -- 1.8.3.1

3 4

[libvirt] [PATCH] qemuDomainRename: Report error if adding domain to domainObj list fails
by Michal Privoznik 17 Aug '15

17 Aug '15

So, domain renaming works like this: new domain name is added into the list of domain objects. Then, domain definition is updated. After that, old domain name is removed from the domain object list. Now, if the very firs step fails for some reason, no error is reported: virsh # domrename dummy dummy error: An error occurred, but the cause is unknown Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3683591..8e365bd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19950,8 +19950,11 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) + if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("could not add new name into internal list of domains")); goto endjob; + } if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) goto rollback; -- 2.4.6

2 7