3:22 p.m.
When storagePoolCreate is called to define a pool that already exists, it mistakenly
destroys the existing pool. This patch fixes the problem.
Dave
3:22 p.m.
New subject: [libvirt] [PATCH 1/1] Fix pool create when pool already exists.
* src/storage_driver.c: don't call virStoragePoolObjRemove when a pool create call
fails because the pool already exists.
---
src/storage_driver.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 69dcbda..5a26993 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -476,6 +476,8 @@ storagePoolCreate(virConnectPtr conn,
if (pool) {
virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
"%s", _("storage pool already
exists"));
+ virStoragePoolObjUnlock(pool);
+ pool = NULL;
goto cleanup;
}
--
1.6.0.6
4:32 p.m.
New subject: [libvirt] [PATCH 1/1] Fix pool create when pool already exists.
On Tue, Jun 30, 2009 at 04:22:38PM -0400, David Allan wrote:
* src/storage_driver.c: don't call virStoragePoolObjRemove when a
pool create call fails because the pool already exists.
---
src/storage_driver.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 69dcbda..5a26993 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -476,6 +476,8 @@ storagePoolCreate(virConnectPtr conn,
if (pool) {
virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
"%s", _("storage pool already
exists"));
+ virStoragePoolObjUnlock(pool);
+ pool = NULL;
goto cleanup;
}
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:37 a.m.
New subject: [libvirt] [PATCH 1/1] Fix pool create when pool already exists.
On Tue, Jun 30, 2009 at 10:32:45PM +0100, Daniel P. Berrange wrote:
On Tue, Jun 30, 2009 at 04:22:38PM -0400, David Allan wrote:
> * src/storage_driver.c: don't call virStoragePoolObjRemove when a pool create
call fails because the pool already exists.
> ---
> src/storage_driver.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/src/storage_driver.c b/src/storage_driver.c
> index 69dcbda..5a26993 100644
> --- a/src/storage_driver.c
> +++ b/src/storage_driver.c
> @@ -476,6 +476,8 @@ storagePoolCreate(virConnectPtr conn,
> if (pool) {
> virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> "%s", _("storage pool already
exists"));
> + virStoragePoolObjUnlock(pool);
> + pool = NULL;
> goto cleanup;
> }
ACK
Indeed, a very serious bug, applied and commited !
thanks,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
4:02 a.m.
On Tue, Jun 30, 2009 at 04:22:37PM -0400, David Allan wrote:
When storagePoolCreate is called to define a pool that already
exists, it mistakenly destroys the existing pool. This patch fixes the problem.
Hum, I tried to assert the level of damage this bug generates and
I have been surprized by the following:
virsh # pool-create /tmp/pool.xml
Pool testpool created from /tmp/pool.xml
virsh # pool-list
Name State Autostart
-----------------------------------------
testpool active no
virsh # pool-create /tmp/pool.xml
error: Failed to create pool from /tmp/pool.xml
error: internal error storage pool already exists
virsh # pool-list
Name State Autostart
-----------------------------------------
testpool active no
virsh #
I.e. on an unpatched libvirtd, I can hit the error, but the pool still
shows up, so I'm wondering what's the effect of the
virStoragePoolObjRemove(&driver->pools, pool);
that we are avoiding here... it should remove the pool from the list,
but I'm not seeing this, so I'm a bit puzzled...
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
4:23 a.m.
On Wed, Jul 01, 2009 at 11:02:08AM +0200, Daniel Veillard wrote:
On Tue, Jun 30, 2009 at 04:22:37PM -0400, David Allan wrote:
> When storagePoolCreate is called to define a pool that already exists, it mistakenly
destroys the existing pool. This patch fixes the problem.
Hum, I tried to assert the level of damage this bug generates and
I have been surprized by the following:
virsh # pool-create /tmp/pool.xml
Pool testpool created from /tmp/pool.xml
virsh # pool-list
Name State Autostart
-----------------------------------------
testpool active no
virsh # pool-create /tmp/pool.xml
error: Failed to create pool from /tmp/pool.xml
error: internal error storage pool already exists
virsh # pool-list
Name State Autostart
-----------------------------------------
testpool active no
virsh #
I.e. on an unpatched libvirtd, I can hit the error, but the pool still
shows up, so I'm wondering what's the effect of the
virStoragePoolObjRemove(&driver->pools, pool);
that we are avoiding here... it should remove the pool from the list,
but I'm not seeing this, so I'm a bit puzzled...
What version of libvirt are you running ? This bug was fairly
recently introduced - it doesn't hit F11 libvirt, but does hit
current CVS when I try your steps. (ie 'testpool' disappears)
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:24 a.m.
On Wed, Jul 01, 2009 at 10:23:45AM +0100, Daniel P. Berrange wrote:
On Wed, Jul 01, 2009 at 11:02:08AM +0200, Daniel Veillard wrote:
> On Tue, Jun 30, 2009 at 04:22:37PM -0400, David Allan wrote:
> > When storagePoolCreate is called to define a pool that already exists, it
mistakenly destroys the existing pool. This patch fixes the problem.
>
> Hum, I tried to assert the level of damage this bug generates and
> I have been surprized by the following:
>
> virsh # pool-create /tmp/pool.xml
> Pool testpool created from /tmp/pool.xml
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh # pool-create /tmp/pool.xml
> error: Failed to create pool from /tmp/pool.xml
> error: internal error storage pool already exists
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh #
>
> I.e. on an unpatched libvirtd, I can hit the error, but the pool still
> shows up, so I'm wondering what's the effect of the
> virStoragePoolObjRemove(&driver->pools, pool);
> that we are avoiding here... it should remove the pool from the list,
> but I'm not seeing this, so I'm a bit puzzled...
What version of libvirt are you running ? This bug was fairly
recently introduced - it doesn't hit F11 libvirt, but does hit
current CVS when I try your steps. (ie 'testpool' disappears)
Hum, I tried an updated RHEL build, that could be the reason, it
appplied fine though and the code was looking similar, I must have
missed something ...
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:02 a.m.
Daniel P. Berrange wrote:
On Wed, Jul 01, 2009 at 11:02:08AM +0200, Daniel Veillard wrote:
> On Tue, Jun 30, 2009 at 04:22:37PM -0400, David Allan wrote:
>> When storagePoolCreate is called to define a pool that already exists, it
mistakenly destroys the existing pool. This patch fixes the problem.
> Hum, I tried to assert the level of damage this bug generates and
> I have been surprized by the following:
>
> virsh # pool-create /tmp/pool.xml
> Pool testpool created from /tmp/pool.xml
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh # pool-create /tmp/pool.xml
> error: Failed to create pool from /tmp/pool.xml
> error: internal error storage pool already exists
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh #
>
> I.e. on an unpatched libvirtd, I can hit the error, but the pool still
> shows up, so I'm wondering what's the effect of the
> virStoragePoolObjRemove(&driver->pools, pool);
> that we are avoiding here... it should remove the pool from the list,
> but I'm not seeing this, so I'm a bit puzzled...
What version of libvirt are you running ? This bug was fairly
recently introduced - it doesn't hit F11 libvirt, but does hit
current CVS when I try your steps. (ie 'testpool' disappears)
Yes, it went in 2009-06-22, so it would have to be very recent code to
hit it, and it does happen 100% of the time.
Dave
5:30 a.m.
Hi,
Believe it is my bad. Didn't check this case when I fixed the problem related to pools
that were not correctly cleaned up when the create failed.
/Henrik
-----Original Message-----
From: libvir-list-bounces(a)redhat.com on behalf of Dave Allan
Sent: Wed 2009-07-01 16:02
To: Daniel P. Berrange
Cc: libvirt-list(a)redhat.com
Subject: Re: [libvirt] [PATCH 0/1] Fix pool create when pool already exists
Daniel P. Berrange wrote:
On Wed, Jul 01, 2009 at 11:02:08AM +0200, Daniel Veillard wrote:
> On Tue, Jun 30, 2009 at 04:22:37PM -0400, David Allan wrote:
>> When storagePoolCreate is called to define a pool that already exists, it
mistakenly destroys the existing pool. This patch fixes the problem.
> Hum, I tried to assert the level of damage this bug generates and
> I have been surprized by the following:
>
> virsh # pool-create /tmp/pool.xml
> Pool testpool created from /tmp/pool.xml
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh # pool-create /tmp/pool.xml
> error: Failed to create pool from /tmp/pool.xml
> error: internal error storage pool already exists
>
> virsh # pool-list
> Name State Autostart
> -----------------------------------------
> testpool active no
>
> virsh #
>
> I.e. on an unpatched libvirtd, I can hit the error, but the pool still
> shows up, so I'm wondering what's the effect of the
> virStoragePoolObjRemove(&driver->pools, pool);
> that we are avoiding here... it should remove the pool from the list,
> but I'm not seeing this, so I'm a bit puzzled...
What version of libvirt are you running ? This bug was fairly
recently introduced - it doesn't hit F11 libvirt, but does hit
current CVS when I try your steps. (ie 'testpool' disappears)
Yes, it went in 2009-06-22, so it would have to be very recent code to
hit it, and it does happen 100% of the time.
Dave
--
Libvir-list mailing list
Libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5435
days inactive
5437
days old
8 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
David Allan
-
Henrik Persson E