[libvirt] [PATCH] qemu: fix the error cover issue in qemuDomainAddCgroupForThread

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@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

On 08/14/2015 02:59 AM, Luyao Huang wrote:
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@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);
Apparently virCgroupRemove can also overwrite a message, see virCgroupNewMachineSystemd for an example of how to save the error message and restore it.. Perhaps all the callers that fail would need a similar sequence John I'm at KVM Forum this week so digging and finding out the answer myself is a challenge with the flakiness of our network connection...
goto error; }

On 08/19/2015 01:40 AM, John Ferlan wrote:
On 08/14/2015 02:59 AM, Luyao Huang wrote:
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@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); Apparently virCgroupRemove can also overwrite a message, see virCgroupNewMachineSystemd for an example of how to save the error message and restore it.. Perhaps all the callers that fail would need a similar sequence
You are right, maybe we could introduce a macro maybe named virErrorAvoidRecover for these case.
John
I'm at KVM Forum this week so digging and finding out the answer myself is a challenge with the flakiness of our network connection...
So lucky you are (of course i mean KVM Forum), i will fix them in another patches. Thanks a lot for your review. Luyao
goto error; }

On 14.08.2015 08:59, Luyao Huang wrote:
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@redhat.com> ---
Reworded the commit message a bit, ACKed and pushed. Michal

On 08/19/2015 02:17 AM, Michal Privoznik wrote:
On 14.08.2015 08:59, Luyao Huang wrote:
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@redhat.com> --- Reworded the commit message a bit, ACKed and pushed.
Thanks a lot for your review and help.
Michal
Luyao
participants (4)
-
John Ferlan
-
lhuang
-
Luyao Huang
-
Michal Privoznik