[libvirt] [PATCH 5/8] UndefineFlags: Implement qemu driver handling

[Eric Blake]

On 07/13/2011 04:19 AM, Osier Yang wrote:
> ---
>  src/qemu/qemu_driver.c |   31 +++++++++++++++++++++++++++----
>  1 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f962e2c..a9f6986 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2402,10 +2402,8 @@ cleanup:
>  static char *
>  qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) {
>      char *ret;
> -    char uuidstr[VIR_UUID_STRING_BUFLEN];
> -    virUUIDFormat(vm->def->uuid, uuidstr);
>  
> -    if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
> +    if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {

This hunk won't apply to existing libvirt.git.  Did you forget to rebase
out your first attempt?

> @@ -4312,6 +4315,20 @@ static int qemudDomainUndefine(virDomainPtr dom) {
>          goto cleanup;
>      }
>  
> +    if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) {
> +        name = qemuDomainManagedSavePath(driver, vm);
> +
> +        if (name == NULL)
> +            goto cleanup;
> +
> +        if (virFileExists(name) && (unlink(name) < 0)) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("failed on removing domain managed state "
> +                            "file '%s'"), name);
> +            goto cleanup;
> +        }
> +    }

I think we need to explicitly reject attempts to undefine a domain while
a state file exists.  That is, I think this logic needs to be:

name = qemuDomainManagedSavePath(driver, vm);
if (name == NULL)
    goto cleanup;
if (virFileExists(name)) {
    if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_STATE) {
        if (unlink(name) < 0) {
            error - failed to remove
        }
    } else {
        error - refusing to undefine with managed state file present
   }
}

Yes, it will change existing behavior (previously, you could undefine a
domain and leave the state file behind), but that was unsafe, and this
is arguably a bug fix.  The default should be as safe as possible, with
the flags (VIR_DOMAIN_UNDEFINE_MANAGED_STATE) in place to make things
faster.

> @@ -8487,6 +8509,7 @@ static virDriver qemuDriver = {
>      .domainCreateWithFlags = qemudDomainStartWithFlags, /* 0.8.2 */
>      .domainDefineXML = qemudDomainDefine, /* 0.2.0 */
>      .domainUndefine = qemudDomainUndefine, /* 0.2.0 */
> +    .domainUndefineWithFlags = qemudDomainUndefineWithFlags, /* 0.9.4 */

Also, I think that for every hypervisor that supports domainUndefine, we
should add a (trivial) domainUndefine[With]Flags wrapper, so that the
new API is available everywhere in 0.9.4, but do that as a separate patch.

I'm about to post a series to add virDomainSaveFlags, which might serve
as an example for what I'm thinking about.  That series will add a
no-semantic-change wrapper in the first commit, and only later changes
the qemu wrapper to learn to honor a new flag value.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110713/c1816a2b/attachment-0001.sig>