[libvirt] [PATCH] Fixed documentation for destroy storage pool
From: Francesc Guasch <frankie@telecos.upc.edu> --- lib/Sys/Virt/StoragePool.pm | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/Sys/Virt/StoragePool.pm b/lib/Sys/Virt/StoragePool.pm index 0bc1d50..2ba5101 100644 --- a/lib/Sys/Virt/StoragePool.pm +++ b/lib/Sys/Virt/StoragePool.pm @@ -115,14 +115,11 @@ C<define_storage_pool> method in L<Sys::Virt>. Remove the configuration associated with a storage pool previously defined with the C<define_storage pool> method in L<Sys::Virt>. If the storage pool is -running, you probably want to use the C<shutdown> or C<destroy> -methods instead. +running, you probably want to use the C<destroy> method instead. =item $pool->destroy() -Immediately terminate the machine, and remove it from the virtual -machine monitor. The C<$pool> handle is invalid after this call -completes and should not be used again. +Immediately stop the storage pool. =item $flag = $pool->get_autostart(); -- 2.14.1
On Sat, Dec 30, 2017 at 09:15:34AM +0100, frankie@telecos.upc.edu wrote:
From: Francesc Guasch <frankie@telecos.upc.edu>
--- lib/Sys/Virt/StoragePool.pm | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/Sys/Virt/StoragePool.pm b/lib/Sys/Virt/StoragePool.pm index 0bc1d50..2ba5101 100644 --- a/lib/Sys/Virt/StoragePool.pm +++ b/lib/Sys/Virt/StoragePool.pm @@ -115,14 +115,11 @@ C<define_storage_pool> method in L<Sys::Virt>.
Remove the configuration associated with a storage pool previously defined with the C<define_storage pool> method in L<Sys::Virt>. If the storage pool is -running, you probably want to use the C<shutdown> or C<destroy> -methods instead. +running, you probably want to use the C<destroy> method instead.
If you want to make the pool unmanaged by libvirt, destroy doesn't help at all since it would only stop a running pool, but wouldn't undefine it. Therefore, we should either omit the sentence completely or use something like this: 'Calling C<undefine> on a running pool makes it transient, thus leaving the underlying object intact, so if object discard is desired, C<destroy> should be called first.' However, truth to be told, even my suggested sentence isn't correct, since undefine on running pools results in an error - we need to fix that since it should behave the same way as domains and make them transient. Maybe we can drop the additional sentence now and update it later when things work the expected way. Erik
On 03/01/18 10:47, Erik Skultety wrote:
On Sat, Dec 30, 2017 at 09:15:34AM +0100, frankie@telecos.upc.edu wrote:
From: Francesc Guasch <frankie@telecos.upc.edu> diff --git a/lib/Sys/Virt/StoragePool.pm b/lib/Sys/Virt/StoragePool.pm index 0bc1d50..2ba5101 100644 --- a/lib/Sys/Virt/StoragePool.pm +++ b/lib/Sys/Virt/StoragePool.pm @@ -115,14 +115,11 @@ C<define_storage_pool> method in L<Sys::Virt>.
Remove the configuration associated with a storage pool previously defined with the C<define_storage pool> method in L<Sys::Virt>. If the storage pool is -running, you probably want to use the C<shutdown> or C<destroy> -methods instead. +running, you probably want to use the C<destroy> method instead.
If you want to make the pool unmanaged by libvirt, destroy doesn't help at all since it would only stop a running pool, but wouldn't undefine it. Therefore, we should either omit the sentence completely or use something like this: 'Calling C<undefine> on a running pool makes it transient, thus leaving the underlying object intact, so if object discard is desired, C<destroy> should be called first.'
Good point. But I use destroy to set the storage pool inactive, it works for me.
However, truth to be told, even my suggested sentence isn't correct, since undefine on running pools results in an error - we need to fix that since it should behave the same way as domains and make them transient. Maybe we can drop the additional sentence now and update it later when things work the expected way.
My initial concern was that shutdown is not in the Storage Pool API. Only destroy can be used. I guess it got pasted from the Domain module.
On Fri, Jan 05, 2018 at 09:47:51AM +0100, Francesc Guasch wrote:
On 03/01/18 10:47, Erik Skultety wrote:
On Sat, Dec 30, 2017 at 09:15:34AM +0100, frankie@telecos.upc.edu wrote:
From: Francesc Guasch <frankie@telecos.upc.edu> diff --git a/lib/Sys/Virt/StoragePool.pm b/lib/Sys/Virt/StoragePool.pm index 0bc1d50..2ba5101 100644 --- a/lib/Sys/Virt/StoragePool.pm +++ b/lib/Sys/Virt/StoragePool.pm @@ -115,14 +115,11 @@ C<define_storage_pool> method in L<Sys::Virt>.
Remove the configuration associated with a storage pool previously defined with the C<define_storage pool> method in L<Sys::Virt>. If the storage pool is -running, you probably want to use the C<shutdown> or C<destroy> -methods instead. +running, you probably want to use the C<destroy> method instead.
If you want to make the pool unmanaged by libvirt, destroy doesn't help at all since it would only stop a running pool, but wouldn't undefine it. Therefore, we should either omit the sentence completely or use something like this: 'Calling C<undefine> on a running pool makes it transient, thus leaving the underlying object intact, so if object discard is desired, C<destroy> should be called first.'
Good point. But I use destroy to set the storage pool inactive, it works for me.
However, truth to be told, even my suggested sentence isn't correct, since undefine on running pools results in an error - we need to fix that since it should behave the same way as domains and make them transient. Maybe we can drop the additional sentence now and update it later when things work the expected way.
My initial concern was that shutdown is not in the Storage Pool API. Only destroy can be used. I guess it got pasted from the Domain module.
Yeah, that's some historical artifact we can't get rid of. Sadly, the naming is very unfortunate. Anyhow, I went ahead and pushed your patch (once we fix the storage pool's behavior we might want to adjust <undefine> again, but for the time being, it's fine). Just a small advice, when sending patches against libvirt derivatives, i.e. perl, python, etc. we use a prefix in the subject, e.g. "[libvirt-<derivative>]...". Also, it's always nice to provide a commit message to explain the reason behind the change. I took care of the commit message and pushed the patch, congratulations on your first contribution. Erik
participants (3)
-
Erik Skultety -
Francesc Guasch -
frankie＠telecos.upc.edu