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

Re: [libvirt] [PATCH RFC 1/3] undefine: Add flags for removing storage files when undefining a domain.



On Thu, Jul 21, 2011 at 05:12:14PM +0200, Peter Krempa wrote:
> Adds option for virDomainUndefineFlags to remove storage associated with
> the domain while undefining it.
> 
> VIR_DOMAIN_UNDEFINE_DISK_FILE - remove disk files, that aren't managed by
> storage pools.
> 
> VIR_DOMAIN_UNDEFINE_DISK_BACKEND - remove storage devices that are managed
> using libvirt storage pools.
> ---
>  include/libvirt/libvirt.h.in |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 40ce0fc..f99bc8b 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1203,6 +1203,8 @@ int                     virDomainUndefine       (virDomainPtr domain);
> 
>  typedef enum {
>      VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 << 0),
> +    VIR_DOMAIN_UNDEFINE_DISK_FILE = (1 << 1),
> +    VIR_DOMAIN_UNDEFINE_DISK_BACKEND = (1 << 2),
> 
>      /* Future undefine control flags should come here. */
>  } virDomainUndefineFlagsValues;

I don't really like us going down this route. While it was easy for
managed save files, when you start thinking about deleting disk storage
life becomes alot more complicated, and you almost certainly need to
make different decisions for different disks in a VM. As such having
a VM level flag for deleting all disks, or not is wrong and IMHO quite
dangerous.

Dealing with failure of the API when using these flags also gets alot
more complicated. If this API fails with these flags set you may or
may not have deleted one or more disks. The application has to then
do a bunch more API calls to try and figure out just which part of
the delete request failed & whether they need to issue further API
calls to clean up the mess. So to actually use these flags and correctly
cope with failure ends up being just as much code as you would have to
write to just call our existing storage volume APIs, or possibly even
more.

NACK to this concept I'm afraid.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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