[libvirt] [PATCH 3/8] storage: Prior to creating a volume, refresh the pool
Ján Tomko
jtomko at redhat.com
Wed Oct 7 14:27:55 UTC 2015
On Mon, Oct 05, 2015 at 07:27:34PM -0400, John Ferlan wrote:
>
>
> On 10/05/2015 07:28 AM, Michal Privoznik wrote:
> > On 02.10.2015 15:41, John Ferlan wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1233003
> >>
> >> Although perhaps bordering on a don't do that type scenario, if
> >> someone creates a volume in a pool outside of libvirt, then uses that
> >> same name to create a volume in the pool via libvirt, then the creation
> >> will fail and in some cases cause the same name volume to be deleted.
> >>
> >> This patch will refresh the pool just prior to checking whether the
> >> named volume exists prior to creating the volume in the pool. While
> >> it's still possible to have a timing window to create a file after the
> >> check - at least we tried. At that point, someone is being malicious.
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >> src/storage/storage_driver.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> >> index 7aaa060..ddf4405 100644
> >> --- a/src/storage/storage_driver.c
> >> +++ b/src/storage/storage_driver.c
> >> @@ -1796,6 +1796,15 @@ storageVolCreateXML(virStoragePoolPtr obj,
> >> if (virStorageVolCreateXMLEnsureACL(obj->conn, pool->def, voldef) < 0)
> >> goto cleanup;
> >>
> >> + /* While not perfect, refresh the list of volumes in the pool and
> >> + * then check that the incoming name isn't already in the pool.
> >> + */
> >> + if (backend->refreshPool) {
> >> + virStoragePoolObjClearVols(pool);
> >> + if (backend->refreshPool(obj->conn, pool) < 0)
> >> + goto cleanup;
> >> + }
> >> +
> >> if (virStorageVolDefFindByName(pool, voldef->name)) {
> >> virReportError(VIR_ERR_STORAGE_VOL_EXIST,
> >> _("'%s'"), voldef->name);
> >>
> >
This is a workaround for the linked bug, not a fix. Refreshing the pool
seems excessive to me.
> > Okay, this makes sense. Not only from the POV you are presenting, but I'd expect my pool to be refreshed after I create new volume in it.
> >
Other volume APIs don't refresh the pool, I think we should be
consistent.
> > And I think we have a way to eliminate even the little window - just track if we successfully built the volume or not. Something among these lines:
> >
After properly propagating the error value if the volume exists, we
don't need this patch refreshing the pool to work around the original
bug.
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151007/db59f6e2/attachment-0001.sig>
More information about the libvir-list
mailing list