[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Only restore security label when saving is successfull.



On 03/25/2011 02:54 AM, Osier Yang wrote:
> "qemudDomainSaveFlag" trys to restore security label even if
> the saving fails, a useless warning will be thowed then, e.g.
> if "doStopVcpus" fails.
> 
> * src/qemu/qemu_driver.c
> ---
>  src/qemu/qemu_driver.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index af897ad..1baee58 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1823,6 +1823,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>      int is_reg = 0;
>      unsigned long long offset;
>      virCgroupPtr cgroup = NULL;
> +    bool saved = false;

You don't need this if we can key off of some other condition.

> 
>      memset(&header, 0, sizeof(header));
>      memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
> @@ -2040,6 +2041,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>      if (rc < 0)
>          goto endjob;
> 
> +    saved = true;
> +

Therefore we don't need this.

>      if ((!bypassSecurityDriver) &&
>          virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
>                                                   vm, path) < 0)

Rather, after the point where you have first attempted
RestoreSavedStateLabel, merely change bypassSecurityDriver to true, to
avoid...

> @@ -2087,7 +2090,7 @@ endjob:
>                               path, vm->def->name, rc);
>              }
> 
> -            if ((!bypassSecurityDriver) &&
> +            if ((!bypassSecurityDriver) && saved &&
>                  virSecurityManagerRestoreSavedStateLabel(driver->securityManager,

a second attempt at restoring the label after the first one.

Besides, your logic looks wrong - you are now attempting the second
restore only if 'saved' is true, where in reality, you want to attempt
the second restore only if the first restore wasn't attempted, or '!saved'.

In other words, I think this one-liner is a better patch:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index af897ad..514ff78 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -2044,6 +2044,7 @@ static int qemudDomainSaveFlag(struct qemud_driver
*driver, virDomainPtr dom,
         virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
                                                  vm, path) < 0)
         VIR_WARN("failed to restore save state label on %s", path);
+    bypassSecurityDriver = true;

     if (cgroup != NULL) {
         rc = virCgroupDenyDevicePath(cgroup, path,


-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]