Re: [libvirt] [PATCH v2 4/8] undefine: Implement internal API for qemu driver

On 07/15/2011 03:06 AM, Osier Yang wrote: > * src/qemu/qemu_driver.c: New call back for qemu_driver, > New function qemudDomainUndefineFlags, and changes on > qemudDomainUndefine. > --- > src/qemu/qemu_driver.c | 35 ++++++++++++++++++++++++++++++++++- > 1 files changed, 34 insertions(+), 1 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 3d4207e..0bf135d 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4303,12 +4303,18 @@ cleanup: > return dom; > } > > -static int qemudDomainUndefine(virDomainPtr dom) { > +static int > +qemudDomainUndefineFlags(virDomainPtr dom, > + unsigned int flags) As long as we're touching this, s/qemudDomain/qemuDomain/ > + name = qemuDomainManagedSavePath(driver, vm); > + if (name == NULL) > + goto cleanup; > + > + if (virFileExists(name)) { > + if ((flags& VIR_DOMAIN_UNDEFINE_MANAGED_STATE) > +&& (unlink(name)< 0)) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("Failed on removing domain managed " Grammar. > + "state file '%s'"), name); No need to expose our internal file name details to the user. > + goto cleanup; > + } else { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, Not an internal error. > + _("Refusing to undefine with managed state " > + "file '%s' exists"), name); Grammar. > + goto cleanup; > + } Logic bug. This fails with "Refusing to undefine" if the unlink() succeeds. Rather, you want:

if (virFileExists(name)) { if (flags& VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { if (unlink(name)< 0) { virReportSystemError(errno, "%s", _("Failed to remove managed state for domain")); goto cleanup; } } else { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Refusing to undefine domain with managed state")); goto cleanup; } } Needs a v3.