On Mon, Aug 11, 2008 at 03:58:41PM -0400, Cole Robinson wrote:
The patch below implements virStorageVolDelete for volumes on a disk pool.
The only interesting thing here is that parted wants a partition number to delete, so we need to peel off the end of the volume's target path which will be of the form '/dev/sda1' or similar (I assume. If not, it's still better than having nothing implemented).
Thanks, Cole
diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c index dac827b..28e0a52 100644 --- a/src/storage_backend_disk.c +++ b/src/storage_backend_disk.c @@ -22,11 +22,13 @@ */
#include <config.h> +#include <string.h>
#include "internal.h" #include "storage_backend_disk.h" #include "util.h" #include "memory.h" +#include "c-ctype.h"
enum { VIR_STORAGE_POOL_DISK_DOS = 0, @@ -416,13 +418,6 @@ virStorageBackendDiskBuildPool(virConnectPtr conn, return 0; }
- -static int -virStorageBackendDiskDeleteVol(virConnectPtr conn, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol, - unsigned int flags); - static int virStorageBackendDiskCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, @@ -486,14 +481,41 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
static int virStorageBackendDiskDeleteVol(virConnectPtr conn, - virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, unsigned int flags ATTRIBUTE_UNUSED) { - /* delete a partition */ - virStorageReportError(conn, VIR_ERR_NO_SUPPORT, - _("Disk pools are not yet supported")); - return -1; + char *part_num = NULL; + int i; + + /* Strip target path (ex. '/dev/sda1') of its partition number */ + for (i = (strlen(vol->target.path)-1); i >= 0; --i) { + if (!c_isdigit(vol->target.path[i])) + break; + part_num = &(vol->target.path[i]); + } + + if (!part_num) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot parse partition number from target " + "path '%s'"), vol->target.path); + return -1; + }
This isn't correct because the target path is not guarenteed to point to the master device name /dev/sda1. The user could have configured it to use a stable path such as /dev/disk/by-uuid/4cb23887-0d02-4e4c-bc95-7599c85afc1a So, you need to first call 'readlink' on the vol->target.path, ignoring EINVAL which you'll get if it wasn't a symlink. Once you've done that you'll need to validate that it is sane by checking that vol->source.devices[0] is a prefix of the resolved pathname. Finally, you can extract the partition number. So something along the lines of char devname[PATH_MAX]; if (readlink(vol->target.path, devname, sizeof(devname)) < 0 && errno != EINVAL) virStorageReportError(...) if (!STRPREFIX(devname, vol->source.devices[0].path)) virStorageReportError(....) part_num = devname + strlen(vol->source.devices[0].path)
+ + /* eg parted /dev/sda rm 2 */ + const char *prog[] = { + PARTED, + pool->def->source.devices[0].path, + "rm", + "--script", + part_num, + NULL, + }; +
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 :|