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

Re: [libvirt] [PATCHv2 1/2] virDomainUndefineFlags: Allow NVRAM unlinking



On 09/12/14 13:33, Michal Privoznik wrote:
> When a domain is undefined, there are options to remove it's
> managed save state or snapshots. However, there's another file
> that libvirt creates per domain: the NVRAM variable store file.
> Make sure that the file is not left behind if the domain is
> undefined.
> 
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  include/libvirt/libvirt.h.in |  2 ++
>  src/qemu/qemu_driver.c       | 20 +++++++++++++++++++-
>  tools/virsh-domain.c         | 20 ++++++++++++++++----
>  tools/virsh.pod              |  6 +++++-
>  4 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 94b942c..3c2a51a 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2257,6 +2257,8 @@ typedef enum {
>      VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
>                                                            then also remove any
>                                                            snapshot metadata */
> +    VIR_DOMAIN_UNDEFINE_NVRAM              = (1 << 2), /* Also remove any
> +                                                          nvram file */
>  
>      /* Future undefine control flags should come here. */
>  } virDomainUndefineFlagsValues;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 917b286..1807715 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6408,7 +6408,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>      virQEMUDriverConfigPtr cfg = NULL;
>  
>      virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
> -                  VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
> +                  VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA |
> +                  VIR_DOMAIN_UNDEFINE_NVRAM, -1);
>  
>      if (!(vm = qemuDomObjFromDomain(dom)))
>          return -1;
> @@ -6457,6 +6458,23 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>          }
>      }
>  
> +    if (!virDomainObjIsActive(vm) &&
> +        vm->def->os.loader && vm->def->os.loader->nvram &&
> +        virFileExists(vm->def->os.loader->nvram)) {
> +        if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                           _("cannot delete inactive domain with nvram"));
> +            goto cleanup;
> +        }
> +
> +        if (unlink(vm->def->os.loader->nvram) < 0) {
> +            virReportSystemError(errno,
> +                                 _("failed to remove nvram: %s"),
> +                                 vm->def->os.loader->nvram);
> +            goto cleanup;
> +        }
> +    }
> +
>      if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
>          goto cleanup;
>  
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 30b3fa9..ecd5283 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -3248,6 +3248,10 @@ static const vshCmdOptDef opts_undefine[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("remove all domain snapshot metadata, if inactive")
>      },
> +    {.name = "nvram",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("remove nvram file, if inactive")
> +    },
>      {.name = NULL}
>  };
>  
> @@ -3270,6 +3274,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>      bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
>      bool wipe_storage = vshCommandOptBool(cmd, "wipe-storage");
>      bool remove_all_storage = vshCommandOptBool(cmd, "remove-all-storage");
> +    bool nvram = vshCommandOptBool(cmd, "nvram");
>      /* Positive if these items exist.  */
>      int has_managed_save = 0;
>      int has_snapshots_metadata = 0;
> @@ -3313,6 +3318,9 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>          flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
>          snapshots_safe = true;
>      }
> +    if (nvram) {
> +        flags |= VIR_DOMAIN_UNDEFINE_NVRAM;
> +    }
>  
>      if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
>          return false;
> @@ -3503,11 +3511,15 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>       * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in 0.9.4, the
>       * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flag was not present
>       * until 0.9.5; skip to piecewise emulation if we couldn't prove
> -     * above that the new API is safe.  */
> -    if (managed_save_safe && snapshots_safe) {
> +     * above that the new API is safe.
> +     * Moreover, only the newer UndefineFlags() API understands
> +     * the VIR_DOMAIN_UNDEFINE_NVRAM flag. So if user has
> +     * specified --nvram we must use the Flags() API. */
> +    if ((managed_save_safe && snapshots_safe) || nvram) {
>          rc = virDomainUndefineFlags(dom, flags);
> -        if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT &&
> -                        last_error->code != VIR_ERR_INVALID_ARG))
> +        if (rc == 0 || nvram ||
> +            (last_error->code != VIR_ERR_NO_SUPPORT &&
> +             last_error->code != VIR_ERR_INVALID_ARG))
>              goto out;
>          vshResetLibvirtError();
>      }
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 60ee515..5d4b12b 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2083,7 +2083,7 @@ Output the device used for the TTY console of the domain. If the information
>  is not available the processes will provide an exit code of 1.
>  
>  =item B<undefine> I<domain> [I<--managed-save>] [I<--snapshots-metadata>]
> -[ {I<--storage> B<volumes> | I<--remove-all-storage>} I<--wipe-storage>]
> +[I<--nvram>] [ {I<--storage> B<volumes> | I<--remove-all-storage>} I<--wipe-storage>]
>  
>  Undefine a domain. If the domain is running, this converts it to a
>  transient domain, without stopping it. If the domain is inactive,
> @@ -2099,6 +2099,10 @@ domain.  Without the flag, attempts to undefine an inactive domain with
>  snapshot metadata will fail.  If the domain is active, this flag is
>  ignored.
>  
> +The I<--nvram> flag ensures no nvram (/domain/os/nvram/) file is
> +left behind. If the domain has an nvram file and the flag is
> +omitted, the undefine will fail.
> +
>  The I<--storage> flag takes a parameter B<volumes>, which is a comma separated
>  list of volume target names or source paths of storage volumes to be removed
>  along with the undefined domain. Volumes can be undefined and thus removed only
> 

Acked-by: Laszlo Ersek <lersek redhat com>

Thanks!
Laszlo


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