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;