[libvirt] [PATCH 1/1] Do not use virtio-serial port 0 for generic ports
Per the discussion in: https://bugzilla.redhat.com/show_bug.cgi?id=670394 The port numbering should start from 1, not 0. We assign maxport + 1, so start maxport at 0. --- src/conf/domain_conf.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d5445a4..08c21e5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5328,7 +5328,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL && chr->info.addr.vioserial.port == 0) { -int maxport = -1; +int maxport = 0; int j; for (j = 0 ; j < i ; j++) { virDomainChrDefPtr thischr = def->channels[j]; -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] RFC: Canonicalize block device paths
I had a look at the problem of finding the devices representing the partitions of a block device, and it's a messy problem. There are a lot of different conventions that are used depending on what kind of device is partitioned. The following patch seems like a promising approach, but before I spend a bunch of time testing that it works in a variety of cases, I thought I should send it out for comment. Dave David Allan (1): RFC: Canonicalize block device paths src/Makefile.am |9 +++-- src/storage/parthelper.c | 35 +-- 2 files changed, 40 insertions(+), 4 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] RFC: Canonicalize block device paths
There are many naming conventions for partitions associated with a block device. Some of the major ones are: /dev/foo -> /dev/foo1 /dev/foo1 -> /dev/foo1p1 /dev/mapper/foo -> /dev/mapper/foop1 /dev/disk/by-path/foo -> /dev/disk/by-path/foo-part1 The universe of possible conventions isn't clear. Rather than trying to understand all possible conventions, this patch divides devices into two groups, device mapper devices and everything else. Device mapper devices seem always to follow the convention of device -> devicep1; everything else is canonicalized. --- src/Makefile.am |9 +++-- src/storage/parthelper.c | 35 +-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index ece18a6..c8fad28 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1037,8 +1037,13 @@ libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) -libvirt_parthelper_LDADD = $(LIBPARTED_LIBS) ../gnulib/lib/libgnu.la -libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS) +libvirt_parthelper_LDADD = \ + $(LIBPARTED_LIBS) \ + $(DEVMAPPER_LIBS) \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + +libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS) $(DEVMAPPER_CFLAGS) endif endif EXTRA_DIST += $(STORAGE_HELPER_DISK_SOURCES) diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c index ca74456..2a70250 100644 --- a/src/storage/parthelper.c +++ b/src/storage/parthelper.c @@ -35,7 +35,12 @@ #include #include #include +#include +#include +#include +#include +#include "util.h" #include "c-ctype.h" /* we don't need to include the full internal.h just for this */ @@ -52,6 +57,18 @@ enum diskCommand { DISK_GEOMETRY }; +static int +is_dm_device(const char *devname) +{ +struct stat buf; + +if (devname && !stat(devname, &buf) && dm_is_dm_major(major(buf.st_rdev))) { +return 1; +} + +return 0; +} + int main(int argc, char **argv) { PedDevice *dev; @@ -59,6 +76,7 @@ int main(int argc, char **argv) PedPartition *part; int cmd = DISK_LAYOUT; const char *path; +char *canonical_path; const char *partsep; if (argc == 3 && STREQ(argv[2], "-g")) { @@ -69,7 +87,20 @@ int main(int argc, char **argv) } path = argv[1]; -partsep = *path && c_isdigit(path[strlen(path)-1]) ? "p" : ""; +if (is_dm_device(path)) { +partsep = "p"; +canonical_path = strdup(path); +if (canonical_path == NULL) { +return 2; +} +} else { +if (virFileResolveLink(path, &canonical_path) != 0) { +return 2; +} + +partsep = *canonical_path && +c_isdigit(canonical_path[strlen(canonical_path)-1]) ? "p" : ""; +} if ((dev = ped_device_get(path)) == NULL) { fprintf(stderr, "unable to access device %s\n", path); @@ -125,7 +156,7 @@ int main(int argc, char **argv) */ if (part->num != -1) { printf("%s%s%d%c%s%c%s%c%llu%c%llu%c%llu%c", - path, partsep, + canonical_path, partsep, part->num, '\0', type, '\0', content, '\0', -- 1.7.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Add multiIQN tests
* Fix broken rng schema * Add test input & output files --- docs/schemas/storagepool.rng | 16 -- tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml | 22 .../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 22 tests/storagepoolxml2xmltest.c |1 + 4 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index cfcf9a6..b911f7c 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -188,12 +188,14 @@ - - - - - - + + + + + + + + @@ -372,7 +374,7 @@ - + diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml b/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml new file mode 100644 index 000..4c77086 --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml @@ -0,0 +1,22 @@ + + multiiqn + e9392370-2917-565e-792c-e057f46512d7 + 0 + 0 + 0 + + + + + + + + +/dev/disk/by-path + + 0700 + 0 + 0 + + + diff --git a/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml b/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml new file mode 100644 index 000..4c77086 --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml @@ -0,0 +1,22 @@ + + multiiqn + e9392370-2917-565e-792c-e057f46512d7 + 0 + 0 + 0 + + + + + + + + +/dev/disk/by-path + + 0700 + 0 + 0 + + + diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 4550407..33a7343 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -95,6 +95,7 @@ mymain(int argc, char **argv) DO_TEST("pool-netfs"); DO_TEST("pool-scsi"); DO_TEST("pool-mpath"); +DO_TEST("pool-iscsi-multiiqn"); return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Add multiIQN XML dump output
The first of these two patches adds XML output for multi-IQN iSCSI pools. The second makes the rng consistent with the existing code and adds regression tests. My RelaxNG skills are rusty, so I would much appreciate a careful review of the rng change. Dave David Allan (2): Add multiiqn XML dump Add multiIQN tests docs/schemas/storagepool.rng | 16 -- src/conf/storage_conf.c|7 ++ tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml | 22 .../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 22 tests/storagepoolxml2xmltest.c |1 + 5 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Add multiiqn XML dump
--- src/conf/storage_conf.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 422e76a..7ed49a1 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -815,6 +815,13 @@ virStoragePoolSourceFormat(virBufferPtr buf, src->name) virBufferVSprintf(buf,"%s\n", src->name); +if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) && +src->initiator.iqn) { +virBufferAddLit(buf,"\n"); +virBufferVSprintf(buf," \n", src->initiator.iqn); +virBufferAddLit(buf,"\n"); +} + if (options->formatToString) { const char *format = (options->formatToString)(src->format); if (!format) { -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Fix leaks in udev device add/remove v3
* This patch is a modification of a patch submitted by Nigel Jones. It fixes several memory leaks on device addition/removal: 1. Free the virNodeDeviceDefPtr in udevAddOneDevice if the return value is non-zero 2. Always release the node device reference after the device has been processed. * Refactored for better readability per the suggestion of clalance --- src/node_device/node_device_udev.c | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 6e3ecd7..73217c5 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1309,13 +1309,14 @@ static int udevAddOneDevice(struct udev_device *device) goto out; } +/* If this is a device change, the old definition will be freed + * and the current definition will take its place. */ nodeDeviceLock(driverState); dev = virNodeDeviceAssignDef(&driverState->devs, def); nodeDeviceUnlock(driverState); if (dev == NULL) { VIR_ERROR(_("Failed to create device for '%s'"), def->name); -virNodeDeviceDefFree(def); goto out; } @@ -1324,6 +1325,10 @@ static int udevAddOneDevice(struct udev_device *device) ret = 0; out: +if (ret != 0) { +virNodeDeviceDefFree(def); +} + return ret; } @@ -1338,15 +1343,17 @@ static int udevProcessDeviceListEntry(struct udev *udev, name = udev_list_entry_get_name(list_entry); device = udev_device_new_from_syspath(udev, name); + if (device != NULL) { if (udevAddOneDevice(device) != 0) { VIR_INFO("Failed to create node device for udev device '%s'", name); } -udev_device_unref(device); ret = 0; } +udev_device_unref(device); + return ret; } @@ -1454,6 +1461,7 @@ static void udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, } out: +udev_device_unref(device); return; } -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Fix udev memory leak v3
Here's a new version of the fix for the udev memory leak based on Chris Lalancette's feedback. I've tested it with valgrind as well as through an extremely long cycle of device additions and removals, so I'm confident in it, but I'd appreciate an ack that it fixes the reported problem. Dave David Allan (1): Fix leaks in udev device add/remove v3 src/node_device/node_device_udev.c | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Update nodedev scsi_host data before use
* It appears that the udev event for HBA creation arrives before the associated sysfs data is fully populated, resulting in bogus data for the nodedev entry until the entry is refreshed. This problem is particularly troublesome when creating NPIV vHBAs because it results in libvirt failing to find the newly created adapter and waiting for the full timeout period before erroneously failing the create operation. This patch forces an update before any attempt to use any scsi_host nodedev entry. --- src/node_device/node_device_driver.c | 19 +++ 1 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 8fb062c..f7e2f69 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -45,23 +45,9 @@ static int update_caps(virNodeDeviceObjPtr dev) virNodeDevCapsDefPtr cap = dev->def->caps; while (cap) { -/* The only cap that currently needs updating is the WWN of FC HBAs. */ +/* The only caps that currently need updating are FC related. */ if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { -if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { -if (read_wwn(cap->data.scsi_host.host, -"port_name", -&cap->data.scsi_host.wwpn) == -1) { -VIR_ERROR(_("Failed to refresh WWPN for host%d"), - cap->data.scsi_host.host); -} - -if (read_wwn(cap->data.scsi_host.host, -"node_name", -&cap->data.scsi_host.wwnn) == -1) { -VIR_ERROR(_("Failed to refresh WWNN for host%d"), - cap->data.scsi_host.host); -} -} +check_fc_host(&dev->def->caps->data); } cap = cap->next; } @@ -239,6 +225,7 @@ nodeDeviceLookupByWWN(virConnectPtr conn, while (cap) { if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { +check_fc_host(&cap->data); if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Update nodedev scsi_host data
While investigating https://bugzilla.redhat.com/show_bug.cgi?id=597998 I noticed that my nodedev-create operations were timing out more often than not. This patch works around a race between udev generating a new device event and the corresponding sysfs entries being populated. This behavior has appeared before, but previously it was limited to incorrect WWNs; now no FC capability appears at all until the nodedev entry is refreshed. David Allan (1): Update nodedev scsi_host data before use src/node_device/node_device_driver.c | 19 +++ 1 files changed, 3 insertions(+), 16 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Fix device destroy return value
--- src/conf/node_device_conf.c |4 ++-- src/node_device/node_device_driver.c |3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 7f2dac8..6583570 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1308,7 +1308,7 @@ virNodeDeviceGetParentHost(const virNodeDeviceObjListPtr devs, parent = virNodeDeviceFindByName(devs, parent_name); if (parent == NULL) { virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not find parent HBA for '%s'"), + _("Could not find parent device for '%s'"), dev_name); ret = -1; goto out; @@ -1328,7 +1328,7 @@ virNodeDeviceGetParentHost(const virNodeDeviceObjListPtr devs, if (cap == NULL) { virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("Parent HBA %s is not capable " + _("Parent device %s is not capable " "of vport operations"), parent->def->name); ret = -1; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index a6c1fa0..8fb062c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -584,7 +584,7 @@ cleanup: static int nodeDeviceDestroy(virNodeDevicePtr dev) { -int ret = 0; +int ret = -1; virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData; virNodeDeviceObjPtr obj = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; @@ -631,6 +631,7 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; } +ret = 0; out: if (obj) virNodeDeviceObjUnlock(obj); -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Fix nodeDeviceDestroy return value
Set nodeDeviceDestroy's return value correctly in failure cases and clarify the error message generated when the parent device is not vport capable. David Allan (1): Fix device destroy return value src/conf/node_device_conf.c |4 ++-- src/node_device/node_device_driver.c |3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Add mkfs and libblkid to build system v2
Change per feedback from Dan B: * Add libblkid dependency to libvirt.spec.in --- configure.ac| 25 + libvirt.spec.in |5 + 2 files changed, 30 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 36ba703..6923ce2 100644 --- a/configure.ac +++ b/configure.ac @@ -43,6 +43,7 @@ DEVMAPPER_REQUIRED=1.0.0 LIBCURL_REQUIRED="7.18.0" LIBPCAP_REQUIRED="1.0.0" LIBNL_REQUIRED="1.1" +LIBBLKID_REQUIRED="2.17" dnl Checks for C compiler. AC_PROG_CC @@ -1309,6 +1310,25 @@ if test "$with_nwfilter" = "yes" ; then fi AM_CONDITIONAL([WITH_NWFILTER], [test "$with_nwfilter" = "yes"]) +dnl libblkid is used by several storage drivers; therefore we probe +dnl for it unconditionally. +AC_ARG_WITH([libblkid], + [AS_HELP_STRING([--with-libblkid], +[use libblkid to scan for filesystems and partitions @<:@default=check@:>@])], + [], + [with_libblkid=check]) + +if test "x$with_libblkid" = "xyes" || test "x$with_libblkid" = "xcheck"; then + PKG_CHECK_MODULES([BLKID], + [blkid >= $LIBBLKID_REQUIRED], + [with_libblkid="yes"], + [with_libblkid="no"]) +fi + +if test x"$with_libblkid" = x"yes"; then + AC_DEFINE([HAVE_LIBBLKID], [1], [libblkid is present]) +fi +AM_CONDITIONAL([HAVE_LIBBLKID], [test x"$with_libblkid" = x"yes"]) AC_ARG_WITH([storage-fs], AC_HELP_STRING([--with-storage-fs], [with FileSystem backend for the storage driver @<:@default=check@:>@]),[],[with_storage_fs=check]) @@ -1342,12 +1362,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([MKFS], [mkfs], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi +if test -z "$MKFS" ; then AC_MSG_ERROR([We need mkfs for FS storage driver]) ; fi else if test -z "$MOUNT" ; then with_storage_fs=no ; fi if test -z "$UMOUNT" ; then with_storage_fs=no ; fi +if test -z "$MKFS" ; then with_storage_fs=no ; fi if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi fi @@ -1358,6 +1381,8 @@ if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then [Location or name of the mount program]) AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], [Location or name of the mount program]) +AC_DEFINE_UNQUOTED([MKFS],["$MKFS"], +[Location or name of the mkfs program]) fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/libvirt.spec.in b/libvirt.spec.in index 6edbf2f..1bb58db 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -228,6 +228,11 @@ BuildRequires: util-linux # For showmount in FS driver (netfs discovery) BuildRequires: nfs-utils Requires: nfs-utils +# For mkfs +Requires: util-linux +# For pool-build probing for existing pools +BuildRequires: libblkid-devel >= 2.17 +Requires: libblkid >= 2.17 # For glusterfs %if 0%{?fedora} >= 11 Requires: glusterfs-client >= 2.0.1 -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Add filesystem pool formatting (v3)
The following patches add the ability to format filesystem pools when the appropriate flags are passed to pool build. As before, I have implemented two new flags: VIR_STORAGE_POOL_BUILD_NO_OVERWRITE causes the build to probe for an existing pool of the requested type. The build operation formats the filesystem if it does not find an existing filesystem of that type. VIR_STORAGE_POOL_BUILD_OVERWRITE causes the build to format unconditionally. These patches incorporate all the feedback received on earlier versions. The incremental is pretty unhelpful, so these are complete replacement patches. Dave David Allan (2): Add mkfs and libblkid to build system v2 Add fs pool formatting v3 configure.ac | 25 + include/libvirt/libvirt.h.in |6 +- include/libvirt/virterror.h |2 + libvirt.spec.in |5 + src/Makefile.am |4 + src/libvirt_private.syms |4 + src/storage/storage_backend_fs.c | 180 +- src/storage/storage_backend_fs.h |9 ++- src/util/virterror.c | 12 +++ tools/virsh.c| 14 +++- 10 files changed, 254 insertions(+), 7 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Add fs pool formatting v3
* This patch adds the ability to make the filesystem for a filesystem pool during a pool build. The patch adds two new flags, no overwrite and overwrite, to control when mkfs gets executed. By default, the patch preserves the current behavior, i.e., if no flags are specified, pool build on a filesystem pool only makes the directory on which the filesystem will be mounted. If the no overwrite flag is specified, the target device is checked to determine if a filesystem of the type specified in the pool is present. If a filesystem of that type is already present, mkfs is not executed and the build call returns an error. Otherwise, mkfs is executed and any data present on the device is overwritten. If the overwrite flag is specified, mkfs is always executed, and any existing data on the target device is overwritten unconditionally. Changes per feedback from eblake: * Made probe & overwrite flags exclusive * Changed LDFLAGS to LIBADD in Makefile.am * Added missing virCheckFlags() * Fixed copyright dates * Removed cast of char * passed to libblkid and replaced it with a strdup'd copy * Changed flags to an unsigned int in virsh.c Changes per feedback from Dan B. * Changed probe flag to no-overwrite * Moved libblkid probe code into storage_backend_fs.c --- include/libvirt/libvirt.h.in |6 +- include/libvirt/virterror.h |2 + src/Makefile.am |4 + src/libvirt_private.syms |4 + src/storage/storage_backend_fs.c | 180 +- src/storage/storage_backend_fs.h |9 ++- src/util/virterror.c | 12 +++ tools/virsh.c| 14 +++- 8 files changed, 224 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 19d5205..075beee 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1126,8 +1126,10 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ - VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_REPAIR = (1 << 0), /* Repair / reinitialize */ + VIR_STORAGE_POOL_BUILD_RESIZE = (1 << 1), /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE = (1 << 2), /* Do not overwrite existing pool */ + VIR_STORAGE_POOL_BUILD_OVERWRITE = (1 << 3) /* Overwrite data */ } virStoragePoolBuildFlags; typedef enum { diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 3bbb293..8d15671 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -163,6 +163,8 @@ typedef enum { VIR_WAR_NO_STORAGE, /* failed to start storage */ VIR_ERR_NO_STORAGE_POOL, /* storage pool not found */ VIR_ERR_NO_STORAGE_VOL, /* storage pool not found */ +VIR_ERR_STORAGE_PROBE_FAILED, /* storage pool probe failed */ +VIR_ERR_STORAGE_PROBE_BUILT, /* storage pool already built */ VIR_WAR_NO_NODE, /* failed to start node driver */ VIR_ERR_INVALID_NODE_DEVICE,/* invalid node device object */ VIR_ERR_NO_NODE_DEVICE,/* node device not found */ diff --git a/src/Makefile.am b/src/Makefile.am index 6bdf73c..bc4db48 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -773,6 +773,10 @@ libvirt_driver_storage_la_LDFLAGS += -module -avoid-version endif libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SOURCES) libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_FS_SOURCES) +if HAVE_LIBBLKID +libvirt_driver_storage_la_CFLAGS += $(BLKID_CFLAGS) +libvirt_driver_storage_la_LIBADD += $(BLKID_LIBS) +endif endif if WITH_STORAGE_LVM diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6cb3d66..abe9a2a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -624,6 +624,10 @@ virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; virStorageFileIsSharedFS; +# storage_backend_fs.h +virStorageBackendFileSystemProbeLibblkid; +virStorageBackendFileSystemProbeDummy; + # threads.h virMutexInit; virMutexDestroy; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index f0cd770..eb083cd 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -34,6 +34,10 @@ #include #include +#if HAVE_LIBBLKID +# include +#endif + #include #include #include @@ -45,6 +49,7 @@ #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -483,6 +488,157 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, #endif /* WITH_STORAGE_FS */ +#if HAVE_LIBBLKID +static virStoragePoolProbeResult +virStorageBackendFileSystemProbe(const char *device, + const char *format) { + +virStoragePoolProbeResult ret = FILESYSTEM_PROBE_ERROR; +blkid_probe probe = NULL; +const char *fstype = NULL; +c
[libvirt] [PATCH 1/1] Improve nodedev parent/child relationships
* If a nodedev has a parent that we don't want to display, we should continue walking up the udev device tree to see if any of its earlier ancestors are devices that we display. It makes the tree much nicer looking than having a whole lot of devices hanging off the root node. --- src/node_device/node_device_udev.c | 54 ++-- 1 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index c437861..6e3ecd7 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1223,31 +1223,43 @@ static int udevSetParent(struct udev_device *device, virNodeDeviceObjPtr dev = NULL; int ret = -1; -parent_device = udev_device_get_parent(device); -if (parent_device == NULL) { -VIR_INFO("Could not find udev parent for device with sysfs path '%s'", - udev_device_get_syspath(device)); -} +parent_device = device; +do { -parent_sysfs_path = udev_device_get_syspath(parent_device); -if (parent_sysfs_path == NULL) { -VIR_INFO("Could not get syspath for parent of '%s'", - udev_device_get_syspath(device)); -parent_sysfs_path = ""; -} +parent_device = udev_device_get_parent(parent_device); +if (parent_device == NULL) { +break; +} -def->parent_sysfs_path = strdup(parent_sysfs_path); -if (def->parent_sysfs_path == NULL) { -virReportOOMError(); -goto out; -} +parent_sysfs_path = udev_device_get_syspath(parent_device); +if (parent_sysfs_path == NULL) { +VIR_INFO("Could not get syspath for parent of '%s'", + udev_device_get_syspath(parent_device)); +} -dev = virNodeDeviceFindBySysfsPath(&driverState->devs, parent_sysfs_path); -if (dev == NULL) { +dev = virNodeDeviceFindBySysfsPath(&driverState->devs, + parent_sysfs_path); +if (dev != NULL) { +def->parent = strdup(dev->def->name); +virNodeDeviceObjUnlock(dev); + +if (def->parent == NULL) { +virReportOOMError(); +goto out; +} + +def->parent_sysfs_path = strdup(parent_sysfs_path); +if (def->parent_sysfs_path == NULL) { +virReportOOMError(); +goto out; +} + +} + +} while (def->parent == NULL && parent_device != NULL); + +if (def->parent == NULL) { def->parent = strdup("computer"); -} else { -def->parent = strdup(dev->def->name); -virNodeDeviceObjUnlock(dev); } if (def->parent == NULL) { -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Improve the nodedev tree relationships
The parent/child relationships in the udev node device backend are really kind of gross. If the parent of a device doesn't exist in the libvirt node device tree, the current code sets the parent to the root "computer" node. The result is that we have a lot of devices hanging directly off the root node and relationships like scsi target to scsi hba are broken. The attached patch simply changes the behavior to keep walking up the udev device tree until it finds an ancestor that exists in the libvirt node device tree, or until it actually reaches the root of the tree. The resulting tree is a lot cleaner and more informative, especially on systems with lots of devices. David Allan (1): Improve nodedev parent/child relationships src/node_device/node_device_udev.c | 54 ++-- 1 files changed, 33 insertions(+), 21 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Add fs pool formatting
irStorageBackend virStorageBackendDirectory; diff --git a/src/storage/storage_backend_fs_libblkid.c b/src/storage/storage_backend_fs_libblkid.c new file mode 100644 index 000..ddd9264 --- /dev/null +++ b/src/storage/storage_backend_fs_libblkid.c @@ -0,0 +1,97 @@ +/* + * storage_backend_fs_libblkid.c: libblkid specific code for + * filesystem backend + * + * Copyright (C) 2007-2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: David Allan + */ + +#include +#include + +#include "virterror_internal.h" +#include "storage_backend_fs.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +virStoragePoolProbeResult +virStorageBackendFileSystemProbeLibblkid(const char *device, + const char *format) { + +virStoragePoolProbeResult ret = FILESYSTEM_PROBE_ERROR; +blkid_probe probe = NULL; +const char *fstype = NULL; +char *names[2]; + +VIR_DEBUG("Probing for existing filesystem of type %s on device %s", + format, device); + +if (blkid_known_fstype(format) == 0) { +virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED, + _("Not capable of probing for " +"filesystem of type %s"), + format); +goto out; +} + +probe = blkid_new_probe_from_filename(device); +if (probe == NULL) { +virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED, + _("Failed to create filesystem probe " + "for device %s"), + device); +goto out; +} + +/* Unfortunately this value comes from the pool definition + * where it is correctly const char, but liblbkid expects it + * to be char, thus the cast. */ +names[0] = (char *)format; +names[1] = NULL; + +blkid_probe_filter_superblocks_type(probe, +BLKID_FLTR_ONLYIN, +names); + +if (blkid_do_probe(probe) != 0) { +VIR_INFO("No filesystem of type '%s' found on device '%s'", + format, device); +ret = FILESYSTEM_PROBE_NOT_FOUND; +} else if (blkid_probe_lookup_value(probe, "TYPE", &fstype, NULL) == 0) { +virStorageReportError(VIR_ERR_STORAGE_PROBE_BUILT, + _("Existing filesystem of type '%s' found on " +"device '%s'"), + fstype, device); +ret = FILESYSTEM_PROBE_FOUND; +} + +if (blkid_do_probe(probe) != 1) { +virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED, + _("Found additional probes to run, " +"filesystem probing may be incorrect")); +ret = FILESYSTEM_PROBE_ERROR; +} + +out: +if (probe != NULL) { +blkid_free_probe(probe); +} + +return ret; +} diff --git a/src/util/virterror.c b/src/util/virterror.c index 96dd1e7..bd845fb 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1019,6 +1019,18 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("Storage volume not found: %s"); break; +case VIR_ERR_STORAGE_PROBE_FAILED: +if (info == NULL) +errmsg = _("Storage pool probe failed"); +else +errmsg = _("Storage pool probe failed: %s"); +break; +case VIR_ERR_STORAGE_PROBE_BUILT: +if (info == NULL) +errmsg = _("Storage pool already built"); +else +errmsg = _("Storage pool already built: %s"); +break; case VIR_ERR_INVALID_STORAGE_POOL: if (info == NULL) errmsg = _("invalid storage pool pointer in"); diff --git a/tools/virsh.c b/tools/virsh.c index 693d409..5f7804a 100644 --- a/tools/virsh.c +
[libvirt] [PATCH 0/2] Filesystem pool probing & filesystem building
The following patches add support for building a filesystem on the source device when building a filesystem pool. The first patch adds mkfs and libblkid to the build system. The second patch adds two flags to virStorageBackendFileSystemBuild. The first flag causes the build operation to probe for an existing filesystem of the type specified in the pool XML. If an existing filesystem of the specified type is present, mkfs is not executed and the build call returns an error. Any other data present on the source device will be overwritten. The second flag causes the build to execute mkfs unconditionally overwriting whatever is currently on the source device. Calling virStorageBackendFileSystemBuild without any flags preserves the current behavior, which is to make the directory on which the filesystem will be mounted, but to leave the source device untouched. David Allan (2): Add mkfs and libblkid to build system Add fs pool formatting configure.ac | 25 include/libvirt/libvirt.h.in |6 +- include/libvirt/virterror.h |2 + libvirt.spec.in |5 ++ po/POTFILES.in|1 + src/Makefile.am |5 ++ src/libvirt_private.syms |4 + src/storage/storage_backend_fs.c | 88 +- src/storage/storage_backend_fs.h | 19 ++ src/storage/storage_backend_fs_libblkid.c | 97 + src/util/virterror.c | 12 tools/virsh.c | 15 - 12 files changed, 273 insertions(+), 6 deletions(-) create mode 100644 src/storage/storage_backend_fs_libblkid.c -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Add mkfs and libblkid to build system
--- configure.ac | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index c187420..b10e4b0 100644 --- a/configure.ac +++ b/configure.ac @@ -42,6 +42,7 @@ HAL_REQUIRED=0.5.0 DEVMAPPER_REQUIRED=1.0.0 LIBCURL_REQUIRED="7.18.0" LIBPCAP_REQUIRED="1.0.0" +LIBBLKID_REQUIRED="2.17" dnl Checks for C compiler. AC_PROG_CC @@ -1308,6 +1309,25 @@ if test "$with_nwfilter" = "yes" ; then fi AM_CONDITIONAL([WITH_NWFILTER], [test "$with_nwfilter" = "yes"]) +dnl libblkid is used by several storage drivers; therefore we probe +dnl for it unconditionally. +AC_ARG_WITH([libblkid], + [AS_HELP_STRING([--with-libblkid], +[use libblkid to scan for filesystems and partitions @<:@default=check@:>@])], + [], + [with_libblkid=check]) + +if test "x$with_libblkid" = "xyes" || test "x$with_libblkid" = "xcheck"; then + PKG_CHECK_MODULES([BLKID], + [blkid >= $LIBBLKID_REQUIRED], + [with_libblkid="yes"], + [with_libblkid="no"]) +fi + +if test x"$with_libblkid" = x"yes"; then + AC_DEFINE([HAVE_LIBBLKID], [1], [libblkid is present]) +fi +AM_CONDITIONAL([HAVE_LIBBLKID], [test x"$with_libblkid" = x"yes"]) AC_ARG_WITH([storage-fs], AC_HELP_STRING([--with-storage-fs], [with FileSystem backend for the storage driver @<:@default=check@:>@]),[],[with_storage_fs=check]) @@ -1341,12 +1361,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([MKFS], [mkfs], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi +if test -z "$MKFS" ; then AC_MSG_ERROR([We need mkfs for FS storage driver]) ; fi else if test -z "$MOUNT" ; then with_storage_fs=no ; fi if test -z "$UMOUNT" ; then with_storage_fs=no ; fi +if test -z "$MKFS" ; then with_storage_fs=no ; fi if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi fi @@ -1357,6 +1380,8 @@ if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then [Location or name of the mount program]) AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], [Location or name of the mount program]) +AC_DEFINE_UNQUOTED([MKFS],["$MKFS"], +[Location or name of the mkfs program]) fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Fix encryption XML indentation
The following patch fixes the storage XML indentation problem caused by my earlier patch to fix domain XML indentation problems. I liked the style of Laine's suggestion of passing in the number of spaces to indent, so I adopted that approach. I also added a regression test for the domain XML. Dave David Allan (1): Fix indentation for storage conf XML src/conf/domain_conf.c |2 +- src/conf/storage_conf.c|2 +- src/conf/storage_encryption_conf.c | 16 +++ src/conf/storage_encryption_conf.h |3 +- .../qemuxml2argv-encrypted-disk.args |1 + .../qemuxml2argv-encrypted-disk.xml| 27 tests/qemuxml2xmltest.c|2 + 7 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Fix indentation for storage conf XML
* virStorageEncryptionFormat is called from both virDomainDiskDefFormat and virStorageVolTargetDefFormat. The proper indentation in the generated XML depends on the caller. My earlier patch to fix the incorrect indentation for the domain XML broke the indentation for the storage XML. This patch adopts Laine's suggestion of requring the caller of virStorageEncryptionFormat to provide an unsigned int with the number of spaces the output should be indented. The patch modifies both callers to provide the additional argument. * Add a regression test for the domain XML --- src/conf/domain_conf.c |2 +- src/conf/storage_conf.c|2 +- src/conf/storage_encryption_conf.c | 16 +++ src/conf/storage_encryption_conf.h |3 +- .../qemuxml2argv-encrypted-disk.args |1 + .../qemuxml2argv-encrypted-disk.xml| 27 tests/qemuxml2xmltest.c|2 + 7 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2de838b..23444fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4957,7 +4957,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " %s\n", def->serial); if (def->encryption != NULL && -virStorageEncryptionFormat(buf, def->encryption) < 0) +virStorageEncryptionFormat(buf, def->encryption, 6) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6467c73..6218e02 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1169,7 +1169,7 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAddLit(buf,"\n"); if (def->encryption != NULL && -virStorageEncryptionFormat(buf, def->encryption) < 0) +virStorageEncryptionFormat(buf, def->encryption, 4) < 0) return -1; virBufferVSprintf(buf, " \n", type); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 7f68d67..7a64050 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -215,7 +215,8 @@ virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root) static int virStorageEncryptionSecretFormat(virBufferPtr buf, - virStorageEncryptionSecretPtr secret) + virStorageEncryptionSecretPtr secret, + unsigned int indent) { const char *type; char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -228,13 +229,15 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, } virUUIDFormat(secret->uuid, uuidstr); -virBufferVSprintf(buf, "\n", type, uuidstr); +virBufferVSprintf(buf, "%*s\n", + indent, "", type, uuidstr); return 0; } int virStorageEncryptionFormat(virBufferPtr buf, - virStorageEncryptionPtr enc) + virStorageEncryptionPtr enc, + unsigned int indent) { const char *format; size_t i; @@ -245,14 +248,15 @@ virStorageEncryptionFormat(virBufferPtr buf, "%s", _("unexpected encryption format")); return -1; } -virBufferVSprintf(buf, " \n", format); +virBufferVSprintf(buf, "%*s\n", + indent, "", format); for (i = 0; i < enc->nsecrets; i++) { -if (virStorageEncryptionSecretFormat(buf, enc->secrets[i]) < 0) +if (virStorageEncryptionSecretFormat(buf, enc->secrets[i], indent + 2) < 0) return -1; } -virBufferAddLit(buf, " \n"); +virBufferVSprintf(buf, "%*s\n", indent, ""); return 0; } diff --git a/src/conf/storage_encryption_conf.h b/src/conf/storage_encryption_conf.h index fd435fc..8309255 100644 --- a/src/conf/storage_encryption_conf.h +++ b/src/conf/storage_encryption_conf.h @@ -67,7 +67,8 @@ void virStorageEncryptionFree(virStorageEncryptionPtr enc); virStorageEncryptionPtr virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root); int virStorageEncryptionFormat(virBufferPtr buf, - virStorageEncryptionPtr enc); + virStorageEncryptionPtr enc, + unsigned int indent); /* A helper for VIR_STORAGE_ENCRYPTION_FORMAT_QCOW */ enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args new file mode 100644 index 000..d8c105
[libvirt] [PATCH 0/1] pushed the following trivial patch
I just pushed the following trivial patch to fix a problem with indentation in the domain XML. The problem was reported as: https://bugzilla.redhat.com/show_bug.cgi?id=573908 Dave David Allan (1): Properly indent encryption tags src/conf/storage_encryption_conf.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Properly indent encryption tags
* Fix for the bug reported at: https://bugzilla.redhat.com/show_bug.cgi?id=573908 --- src/conf/storage_encryption_conf.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index ed71688..7f68d67 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -228,7 +228,7 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, } virUUIDFormat(secret->uuid, uuidstr); -virBufferVSprintf(buf, " \n", type, uuidstr); +virBufferVSprintf(buf, "\n", type, uuidstr); return 0; } @@ -245,14 +245,14 @@ virStorageEncryptionFormat(virBufferPtr buf, "%s", _("unexpected encryption format")); return -1; } -virBufferVSprintf(buf, "\n", format); +virBufferVSprintf(buf, " \n", format); for (i = 0; i < enc->nsecrets; i++) { if (virStorageEncryptionSecretFormat(buf, enc->secrets[i]) < 0) return -1; } -virBufferAddLit(buf, "\n"); +virBufferAddLit(buf, " \n"); return 0; } -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Add enospace option to qemu disk error policy
* Dan Kenigsberg requested explicit support for the qemu default disk error policy which is enospace --- docs/schemas/domain.rng|1 + src/conf/domain_conf.c |3 +- src/conf/domain_conf.h |1 + src/qemu/qemu_conf.c |2 + tests/qemuargv2xmltest.c |3 ++ ...uxml2argv-disk-drive-error-policy-enospace.args |1 + ...muxml2argv-disk-drive-error-policy-enospace.xml | 32 7 files changed, 42 insertions(+), 1 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 58c9fcb..56b6705 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -629,6 +629,7 @@ stop ignore +enospace diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3cd43eb..2de838b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -129,7 +129,8 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, "default", "stop", - "ignore") + "ignore", + "enospace") VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5c64a47..82f2d15 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -156,6 +156,7 @@ enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, VIR_DOMAIN_DISK_ERROR_POLICY_STOP, VIR_DOMAIN_DISK_ERROR_POLICY_IGNORE, +VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE, VIR_DOMAIN_DISK_ERROR_POLICY_LAST }; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index df57d88..48252a5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -4938,6 +4938,8 @@ qemuParseCommandLineDisk(const char *val, def->error_policy = VIR_DOMAIN_DISK_ERROR_POLICY_STOP; else if (STREQ(values[i], "ignore")) def->error_policy = VIR_DOMAIN_DISK_ERROR_POLICY_IGNORE; +else if (STREQ(values[i], "enospace")) +def->error_policy = VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE; } else if (STREQ(keywords[i], "index")) { if (virStrToLong_i(values[i], NULL, 10, &idx) < 0) { virDomainDiskDefFree(def); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index b330238..bd81018 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -165,6 +165,9 @@ mymain(int argc, char **argv) DO_TEST("disk-drive-error-policy-stop", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_MONITOR_JSON | QEMUD_CMD_FLAG_DRIVE_FORMAT); +DO_TEST("disk-drive-error-policy-enospace", QEMUD_CMD_FLAG_DRIVE | +QEMUD_CMD_FLAG_MONITOR_JSON | +QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v2-wt", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_CACHE_V2); DO_TEST("disk-drive-cache-v2-wb", QEMUD_CMD_FLAG_DRIVE | diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args new file mode 100644 index 000..c208821 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=qcow2,cache=off,werror=enospace,rerror=enospace -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,bus=1,unit=0,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.xml new file mode 100644 index 000..8fe64d4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.xml @@ -0,0 +1,32 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219200 + 219200 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu + + + + + + + + + + + + + + + + -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] enospace disk error policy
Dan Kenigsberg requested that we add an option to explicitly request enospace as the disk error policy. David Allan (1): Add enospace option to qemu disk error policy docs/schemas/domain.rng|1 + src/conf/domain_conf.c |3 +- src/conf/domain_conf.h |1 + src/qemu/qemu_conf.c |2 + tests/qemuargv2xmltest.c |3 ++ ...uxml2argv-disk-drive-error-policy-enospace.args |1 + ...muxml2argv-disk-drive-error-policy-enospace.xml | 32 7 files changed, 42 insertions(+), 1 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-enospace.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Variable length structure allocator
I've had this patch hanging around for a while, and I think it's worth committing even though the original reason for it went away. The kind of structure it allocates is reasonably common, and the oversize calculation is tricky to get right. Since we've already done the work (thanks Jim for the oversize calculation) I think it's worth keeping. Dave David Allan (1): Implement variable length structure allocator src/util/memory.c | 40 src/util/memory.h | 35 +++ 2 files changed, 75 insertions(+), 0 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Implement variable length structure allocator
* This patch implements a memory allocator to obtain memory for structures whose last member is a variable length array. C99 refers to these variable length objects as structs containing flexible array members. --- src/util/memory.c | 40 src/util/memory.h | 35 +++ 2 files changed, 75 insertions(+), 0 deletions(-) diff --git a/src/util/memory.c b/src/util/memory.c index b2ee376..dd1216b 100644 --- a/src/util/memory.c +++ b/src/util/memory.c @@ -165,6 +165,46 @@ int virReallocN(void *ptrptr, size_t size, size_t count) } /** + * Vir_Alloc_Var: + * @ptrptr: pointer to hold address of allocated memory + * @struct_size: size of initial struct + * @element_size: size of array elements + * @count: number of array elements to allocate + * + * Allocate struct_size bytes plus an array of 'count' elements, each + * of size element_size. This sort of allocation is useful for + * receiving the data of certain ioctls and other APIs which return a + * struct in which the last element is an array of undefined length. + * The caller of this type of API is expected to know the length of + * the array that will be returned and allocate a suitable buffer to + * contain the returned data. C99 refers to these variable length + * objects as structs containing flexible array members. + * + * Returns -1 on failure, 0 on success + */ +int virAllocVar(void *ptrptr, size_t struct_size, size_t element_size, size_t count) +{ +size_t alloc_size = 0; + +#if TEST_OOM +if (virAllocTestFail()) +return -1; +#endif + +if (VIR_ALLOC_VAR_OVERSIZED(struct_size, count, element_size)) { +errno = ENOMEM; +return -1; +} + +alloc_size = struct_size + (element_size * count); +*(void **)ptrptr = calloc(1, alloc_size); +if (*(void **)ptrptr == NULL) +return -1; +return 0; +} + + +/** * virFree: * @ptrptr: pointer to pointer for address of memory to be freed * diff --git a/src/util/memory.h b/src/util/memory.h index 0228173..c717610 100644 --- a/src/util/memory.h +++ b/src/util/memory.h @@ -48,6 +48,10 @@ int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK; int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; int virReallocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK; +int virAllocVar(void *ptrptr, +size_t struct_size, +size_t element_size, +size_t count) ATTRIBUTE_RETURN_CHECK; void virFree(void *ptrptr); /** @@ -88,6 +92,37 @@ void virFree(void *ptrptr); */ # define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) +/* + * VIR_ALLOC_VAR_OVERSIZED: + * @M: size of base structure + * @N: number of array elements in trailing array + * @S: size of trailing array elements + * + * Check to make sure that the requested allocation will not cause + * arithmetic overflow in the allocation size. The check is + * essentially the same as that in gnulib's xalloc_oversized. + */ +#define VIR_ALLOC_VAR_OVERSIZED(M, N, S) size_t)-1) - (M)) / (S) < (N)) + +/** + * VIR_ALLOC_VAR: + * @ptr: pointer to hold address of allocated memory + * @type: element type of trailing array + * @count: number of array elements to allocate + * + * Allocate sizeof(*ptr) bytes plus an array of 'count' elements, each + * sizeof('type'). This sort of allocation is useful for receiving + * the data of certain ioctls and other APIs which return a struct in + * which the last element is an array of undefined length. The caller + * of this type of API is expected to know the length of the array + * that will be returned and allocate a suitable buffer to contain the + * returned data. C99 refers to these variable length objects as + * structs containing flexible array members. + + * Returns -1 on failure, 0 on success + */ +#define VIR_ALLOC_VAR(ptr, type, count) virAllocVar(&(ptr), sizeof(*ptr), sizeof(type), (count)) + /** * VIR_FREE: * @ptr: pointer holding address to be freed -- 1.7.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] port profile id
The following proof of concept patch attempts to show how we might support the use of port profile IDs. The ID would be associated with an interface on a VM and provided to the network infrastructure at VM start time. Since the interfaces by which the id can be provided are still in flux, the use function is only a stub. Dave David Allan (1): Initial POC of port profile id support docs/schemas/domain.rng |8 src/conf/domain_conf.c | 12 src/conf/domain_conf.h |1 + src/libvirt_private.syms |3 +++ src/qemu/qemu_conf.c | 12 src/util/macvtap.c | 13 + src/util/macvtap.h |4 7 files changed, 53 insertions(+), 0 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Initial POC of port profile id support
--- docs/schemas/domain.rng |8 src/conf/domain_conf.c | 12 src/conf/domain_conf.h |1 + src/libvirt_private.syms |3 +++ src/qemu/qemu_conf.c | 12 src/util/macvtap.c | 13 + src/util/macvtap.h |4 7 files changed, 53 insertions(+), 0 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d804301..5917f60 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -794,6 +794,11 @@ + + + + + @@ -1647,6 +1652,9 @@ (vepa|bridge|private) + + + ([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2} diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66aa53e..ea2f8dd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -479,6 +479,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def) case VIR_DOMAIN_NET_TYPE_DIRECT: VIR_FREE(def->data.direct.linkdev); +VIR_FREE(def->data.direct.profileid); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -1801,6 +1802,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *internal = NULL; char *devaddr = NULL; char *mode = NULL; +char *profileid = NULL; virNWFilterHashTablePtr filterparams = NULL; if (VIR_ALLOC(def) < 0) { @@ -1843,6 +1845,7 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); +profileid = virXMLPropString(cur, "profileid"); } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -2018,6 +2021,10 @@ virDomainNetDefParseXML(virCapsPtr caps, } else def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA; +if (profileid != NULL) { +def->data.direct.profileid = profileid; +} + def->data.direct.linkdev = dev; dev = NULL; @@ -2083,6 +2090,7 @@ cleanup: VIR_FREE(internal); VIR_FREE(devaddr); VIR_FREE(mode); +VIR_FREE(profileid); virNWFilterHashTableFree(filterparams); return def; @@ -5080,6 +5088,10 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.direct.linkdev); virBufferVSprintf(buf, " mode='%s'", virDomainNetdevMacvtapTypeToString(def->data.direct.mode)); +if (def->data.direct.profileid) { +virBufferEscapeString(buf, " profileid='%s'", + def->data.direct.profileid); +} virBufferAddLit(buf, "/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ed1a4ad..4c23a23 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -289,6 +289,7 @@ struct _virDomainNetDef { struct { char *linkdev; int mode; +char *profileid; } direct; } data; char *ifname; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc943f8..a14b114 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -702,3 +702,6 @@ virXPathLongLong; virXPathULongLong; virXPathLongHex; virXPathULongHex; + +# macvtap.h +sendPortProfileId; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8f6f7ec..f2880d3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1469,6 +1469,18 @@ qemudPhysIfaceConnect(virConnectPtr conn, net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; +/* Failure here is equivalent to the failure to plug in a physical + * network port. + * + * If this operation is non-blocking we will have a race between + * the VM starting and the interface coming up. + * + * If any of the subsequent operations fail, will we need to + * unwind the sending of the port profile id? */ +sendPortProfileId(net->data.direct.linkdev, + net->data.direct.mode, + net->data.direct.profileid); + rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode, &res_ifname, vnet_hdr); if (rc >= 0) { diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 999e670..585e56b 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -43,6 +43,7 @@ # include "util.h" # include "memory.h" +# include "logging.h" # include "macvtap.h" # include "conf/domain_conf.h" # include "virterror_internal.h" @@ -673,6 +674,18 @@ configMacvtapTap(int tapfd, int vnet_hdr) } +int +sendPortProfileId(const char *linkdev, + int mode, + const ch
[libvirt] [PATCH 1/1] Add disk error policy to domain XML
* Fixes per feedback from Dan and Daniel * Added test datafiles * Re-disabled JSON flags * Added code to print the error policy attribute when generating XML --- docs/schemas/domain.rng| 12 +++- src/conf/domain_conf.c | 18 +++ src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms |2 +- src/qemu/qemu_conf.c | 17 +- tests/qemuargv2xmltest.c |3 ++ .../qemuxml2argv-disk-drive-error-policy-stop.args |1 + .../qemuxml2argv-disk-drive-error-policy-stop.xml | 32 tests/qemuxml2argvtest.c |3 ++ 9 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 5a8c82b..b276da7 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -521,7 +521,9 @@ - + + + @@ -543,6 +545,14 @@ + + + +stop +ignore + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ffbb26f..17f6220 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -123,6 +123,11 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, "writethrough", "writeback") +VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, + "default", + "stop", + "ignore") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -1295,6 +1300,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, char *target = NULL; char *bus = NULL; char *cachetag = NULL; +char *error_policy = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -1360,6 +1366,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, driverName = virXMLPropString(cur, "name"); driverType = virXMLPropString(cur, "type"); cachetag = virXMLPropString(cur, "cache"); +error_policy = virXMLPropString(cur, "error_policy"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -1476,6 +1483,13 @@ virDomainDiskDefParseXML(xmlNodePtr node, goto error; } +if (error_policy && +(def->error_policy = virDomainDiskErrorPolicyTypeFromString(error_policy)) < 0) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk error policy '%s'"), error_policy); +goto error; +} + if (devaddr) { if (sscanf(devaddr, "%x:%x:%x", &def->info.addr.pci.domain, @@ -1518,6 +1532,7 @@ cleanup: VIR_FREE(driverType); VIR_FREE(driverName); VIR_FREE(cachetag); +VIR_FREE(error_policy); VIR_FREE(devaddr); VIR_FREE(serial); virStorageEncryptionFree(encryption); @@ -4616,6 +4631,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *device = virDomainDiskDeviceTypeToString(def->device); const char *bus = virDomainDiskBusTypeToString(def->bus); const char *cachemode = virDomainDiskCacheTypeToString(def->cachemode); +const char *error_policy = virDomainDiskErrorPolicyTypeToString(def->error_policy); if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -4650,6 +4666,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferVSprintf(buf, " type='%s'", def->driverType); if (def->cachemode) virBufferVSprintf(buf, " cache='%s'", cachemode); +if (def->error_policy) +virBufferVSprintf(buf, " error_policy='%s'", error_policy); virBufferVSprintf(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index efd5db6..e275d35 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -150,6 +150,14 @@ enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_LAST }; +enum virDomainDiskErrorPolicy { +VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, +VIR_DOMAIN_DISK_ERROR_POLICY_STOP, +VIR_DOMAIN_DISK_ERROR_POLICY_IGNORE, + +VIR_DOMAIN_DISK_ERROR_POLICY_LAST +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -163,6 +171,7 @@ struct _virDomainDiskDef { char *driverType; char *serial; int cachemode; +int error_policy; unsigned int readonly : 1; unsigned int sh
[libvirt] [PATCH 0/1] Disk error policy
Here's a revised patch for disk error policy XML incorporating the feedback from Dan and Daniel. Dave David Allan (1): Add disk error policy to domain XML docs/schemas/domain.rng| 12 +++- src/conf/domain_conf.c | 18 +++ src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms |2 +- src/qemu/qemu_conf.c | 17 +- tests/qemuargv2xmltest.c |3 ++ .../qemuxml2argv-disk-drive-error-policy-stop.args |1 + .../qemuxml2argv-disk-drive-error-policy-stop.xml | 32 tests/qemuxml2argvtest.c |3 ++ 9 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Disk error policy
The following patch adds support for setting the disk error policy to the domain XML and using it when setting up the qemu command line. It specifies the werror=somepolicy,rerror=somepolicy flags to tell qemu what to do in the case of an io error. This patch is my first foray into the setup of the qemu command line, so please review with heightened care. Dave David Allan (1): Add disk error policy to domain XML docs/schemas/domain.rng | 12 +++- src/conf/domain_conf.c | 15 +++ src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms |2 +- src/qemu/qemu_conf.c | 12 +--- tests/qemuhelptest.c |1 + tests/qemuxml2argvtest.c |3 +++ 7 files changed, 50 insertions(+), 5 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Add disk error policy to domain XML
--- docs/schemas/domain.rng | 12 +++- src/conf/domain_conf.c | 15 +++ src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms |2 +- src/qemu/qemu_conf.c | 12 +--- tests/qemuhelptest.c |1 + tests/qemuxml2argvtest.c |3 +++ 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 5a8c82b..b276da7 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -521,7 +521,9 @@ - + + + @@ -543,6 +545,14 @@ + + + +stop +ignore + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56c5239..6e90561 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -122,6 +122,11 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, "writethrough", "writeback") +VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, + "default", + "stop", + "ignore") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -1288,6 +1293,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, char *target = NULL; char *bus = NULL; char *cachetag = NULL; +char *error_policy = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -1353,6 +1359,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, driverName = virXMLPropString(cur, "name"); driverType = virXMLPropString(cur, "type"); cachetag = virXMLPropString(cur, "cache"); +error_policy = virXMLPropString(cur, "error_policy"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -1469,6 +1476,13 @@ virDomainDiskDefParseXML(xmlNodePtr node, goto error; } +if (error_policy && +(def->error_policy = virDomainDiskErrorPolicyTypeFromString(error_policy)) < 0) { +virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk error policy '%s'"), error_policy); +goto error; +} + if (devaddr) { if (sscanf(devaddr, "%x:%x:%x", &def->info.addr.pci.domain, @@ -1510,6 +1524,7 @@ cleanup: VIR_FREE(driverType); VIR_FREE(driverName); VIR_FREE(cachetag); +VIR_FREE(error_policy); VIR_FREE(devaddr); VIR_FREE(serial); virStorageEncryptionFree(encryption); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0540a77..b39ab38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -150,6 +150,14 @@ enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_LAST }; +enum virDomainDiskErrorPolicy { +VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, +VIR_DOMAIN_DISK_ERROR_POLICY_STOP, +VIR_DOMAIN_DISK_ERROR_POLICY_IGNORE, + +VIR_DOMAIN_DISK_ERROR_POLICY_LAST +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -163,6 +171,7 @@ struct _virDomainDiskDef { char *driverType; char *serial; int cachemode; +int error_policy; unsigned int readonly : 1; unsigned int shared : 1; virDomainDeviceInfo info; @@ -935,6 +944,7 @@ VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) +VIR_ENUM_DECL(virDomainDiskErrorPolicy) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainFS) VIR_ENUM_DECL(virDomainNet) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5ee23d..0cca3c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -191,7 +191,7 @@ virDomainDefClearDeviceAliases; virDomainDeviceInfoIterate; virDomainClockOffsetTypeToString; virDomainClockOffsetTypeFromString; - +virDomainDiskErrorPolicyTypeToString; # domain_event.h virDomainEventCallbackListAdd; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f2d36f7..0b71ca9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help, /* Keep disabled till we're actually ready to turn on JSON mode * The plan is todo it in 0.13.0 QEMU, but lets wait & see... */ -#if 0 -if (version >= 13000) +if (version >= 12000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; -#endif return flags; } @@ -2448,6 +2446,14 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferAddLit(&opt, ",cache=off"); } +if (qemuCmdFlags & QEMUD_CMD_FLAG_MONITOR_JSON) { +if (disk->error_policy) { +
[libvirt] [PATCH 7/9] Implement the remote dispatch bits of vol wiping
* I had to remove daemon/remote_dispatch* and do a make remote.c in daemon in order to get the dispatch files regenerated properly. The make was failing before I did that. --- daemon/remote.c | 32 daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 daemon/remote_dispatch_table.h |5 + 4 files changed, 46 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 7c4339f..178b03e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4273,6 +4273,38 @@ remoteDispatchStorageVolDelete (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchStorageVolWipe(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_storage_vol_wipe_args *args, + void *ret ATTRIBUTE_UNUSED) +{ +int retval = -1; +virStorageVolPtr vol; + +vol = get_nonnull_storage_vol(conn, args->vol); +if (vol == NULL) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +if (virStorageVolWipe(vol, args->flags) == -1) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +retval = 0; + +out: +if (vol != NULL) { +virStorageVolFree(vol); +} +return retval; +} + +static int remoteDispatchStorageVolGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index f97155b..32a6d78 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -140,3 +140,4 @@ remote_cpu_baseline_args val_remote_cpu_baseline_args; remote_domain_get_job_info_args val_remote_domain_get_job_info_args; remote_domain_abort_job_args val_remote_domain_abort_job_args; +remote_storage_vol_wipe_args val_remote_storage_vol_wipe_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index b81c8c3..4c944f6 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -1298,6 +1298,14 @@ static int remoteDispatchStorageVolLookupByPath( remote_error *err, remote_storage_vol_lookup_by_path_args *args, remote_storage_vol_lookup_by_path_ret *ret); +static int remoteDispatchStorageVolWipe( +struct qemud_server *server, +struct qemud_client *client, +virConnectPtr conn, +remote_message_header *hdr, +remote_error *err, +remote_storage_vol_wipe_args *args, +void *ret); static int remoteDispatchSupportsFeature( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index 5ad6bff..28e7d9a 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -827,3 +827,8 @@ .args_filter = (xdrproc_t) xdr_remote_domain_abort_job_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* StorageVolWipe => 165 */ +.fn = (dispatch_fn) remoteDispatchStorageVolWipe, +.args_filter = (xdrproc_t) xdr_remote_storage_vol_wipe_args, +.ret_filter = (xdrproc_t) xdr_void, +}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] Simplified version of volume wiping based on feedback from the list.
--- src/storage/storage_driver.c | 224 ++ 1 files changed, 224 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ca2540f..1961fa0 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include + #if HAVE_PWD_H # include #endif @@ -1518,6 +1521,226 @@ cleanup: return ret; } + +/* If the volume we're wiping is already a sparse file, we simply + * truncate and extend it to its original size, filling it with + * zeroes. This behavior is guaranteed by POSIX: + * + * http://www.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html + * + * If fildes refers to a regular file, the ftruncate() function shall + * cause the size of the file to be truncated to length. If the size + * of the file previously exceeded length, the extra data shall no + * longer be available to reads on the file. If the file previously + * was smaller than this size, ftruncate() shall increase the size of + * the file. If the file size is increased, the extended area shall + * appear as if it were zero-filled. + */ +static int +storageVolumeZeroSparseFile(virStorageVolDefPtr vol, +off_t size, +int fd) +{ +int ret = -1; +char errbuf[64]; + +ret = ftruncate(fd, 0); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to 0 bytes: '%s'"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +ret = ftruncate(fd, size); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to %ju bytes: '%s'\n"), + vol->target.path, (intmax_t)size, + virStrerror(errno, errbuf, sizeof(errbuf))); +} + +out: +return ret; +} + + +static int +storageWipeExtent(virStorageVolDefPtr vol, + int fd, + off_t extent_start, + off_t extent_length, + char *writebuf, + size_t writebuf_length, + size_t *bytes_wiped) +{ +int ret = -1, written = 0; +off_t remaining = 0; +size_t write_size = 0; +char errbuf[64]; + +VIR_DEBUG("extent logical start: %ju len: %ju", + (intmax_t)extent_start, (intmax_t)extent_length); + +if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) { +virReportSystemError(ret, + _("Failed to seek to position %ju in volume " + "with path '%s': '%s'"), + (intmax_t)extent_start, vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +remaining = extent_length; +while (remaining > 0) { + +write_size = (writebuf_length < remaining) ? writebuf_length : remaining; +written = safewrite(fd, writebuf, write_size); +if (written < 0) { +virReportSystemError(written, + _("Failed to write to storage volume with " + "path '%s': '%s' " + "(attempted to write %zu bytes)"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf)), + write_size); +goto out; +} + +*bytes_wiped += written; +remaining -= written; +} + +VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", + *bytes_wiped, vol->target.path); + +ret = 0; + +out: +return ret; +} + + +static int +storageVolumeWipeInternal(virStorageVolDefPtr def) +{ +int ret = -1, fd = -1; +char errbuf[64]; +struct stat st; +char *writebuf = NULL; +size_t bytes_wiped = 0; + +VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + +fd = open(def->target.path, O_RDWR); +if (fd == -1) { +VIR_ERROR("Failed to open storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (fstat(fd, &st) == -1) { +VIR_ERROR("Failed to stat storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { +ret = storageVolumeZeroSparseFile(def, st.st_size, fd); +} else { + +if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { +virReportOOM
[libvirt] [PATCH 4/9] Implement the public API for vol wiping
--- src/libvirt.c | 47 +++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 1d9b878..74b075b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8479,6 +8479,53 @@ error: /** + * virStorageVolWipe: + * @vol: pointer to storage volume + * @flags: future flags, use 0 for now + * + * Ensure data previously on a volume is not accessible to future reads + * + * Returns 0 on success, or -1 on error + */ +int +virStorageVolWipe(virStorageVolPtr vol, + unsigned int flags) +{ +virConnectPtr conn; +VIR_DEBUG("vol=%p, flags=%u", vol, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { +virLibStorageVolError(NULL, VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); +virDispatchError(NULL); +return (-1); +} + +conn = vol->conn; +if (conn->flags & VIR_CONNECT_RO) { +virLibStorageVolError(vol, VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (conn->storageDriver && conn->storageDriver->volWipe) { +int ret; +ret = conn->storageDriver->volWipe(vol, flags); +if (ret < 0) { +goto error; +} +return ret; +} + +virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(vol->conn); +return -1; +} + + +/** * virStorageVolFree: * @vol: pointer to storage volume * -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/9] Add vol wiping to ESX storage driver struct
--- src/esx/esx_storage_driver.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index b920f3b..7b073a6 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -102,6 +102,7 @@ static virStorageDriver esxStorageDriver = { NULL, /* volCreateXML */ NULL, /* volCreateXMLFrom */ NULL, /* volDelete */ +NULL, /* volWipe */ NULL, /* volGetInfo */ NULL, /* volGetXMLDesc */ NULL, /* volGetPath */ -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] Virsh support for vol wiping
--- tools/virsh.c | 42 ++ 1 files changed, 42 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index aa85ee6..37ff471 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5271,6 +5271,47 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd) /* + * "vol-wipe" command + */ +static const vshCmdInfo info_vol_wipe[] = { +{"help", gettext_noop("wipe a vol")}, +{"desc", gettext_noop("Ensure data previously on a volume is not accessible to future reads")}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_wipe[] = { +{"pool", VSH_OT_STRING, 0, gettext_noop("pool name or uuid")}, +{"vol", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("vol name, key or path")}, +{NULL, 0, 0, NULL} +}; + +static int +cmdVolWipe(vshControl *ctl, const vshCmd *cmd) +{ +virStorageVolPtr vol; +int ret = TRUE; +char *name; + +if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) +return FALSE; + +if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) { +return FALSE; +} + +if (virStorageVolWipe(vol, 0) == 0) { +vshPrint(ctl, _("Vol %s wiped\n"), name); +} else { +vshError(ctl, _("Failed to wipe vol %s"), name); +ret = FALSE; +} + +virStorageVolFree(vol); +return ret; +} + + +/* * "vol-info" command */ static const vshCmdInfo info_vol_info[] = { @@ -7809,6 +7850,7 @@ static const vshCmdDef commands[] = { {"vol-create-as", cmdVolCreateAs, opts_vol_create_as, info_vol_create_as}, {"vol-clone", cmdVolClone, opts_vol_clone, info_vol_clone}, {"vol-delete", cmdVolDelete, opts_vol_delete, info_vol_delete}, +{"vol-wipe", cmdVolWipe, opts_vol_wipe, info_vol_wipe}, {"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml}, {"vol-info", cmdVolInfo, opts_vol_info, info_vol_info}, {"vol-list", cmdVolList, opts_vol_list, info_vol_list}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] Define the internal driver API for vol wiping
--- src/driver.h |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index a64bba0..1a511eb 100644 --- a/src/driver.h +++ b/src/driver.h @@ -723,6 +723,10 @@ typedef int unsigned int flags); typedef int +(*virDrvStorageVolWipe) (virStorageVolPtr vol, + unsigned int flags); + +typedef int (*virDrvStorageVolGetInfo) (virStorageVolPtr vol, virStorageVolInfoPtr info); typedef char * @@ -791,6 +795,7 @@ struct _virStorageDriver { virDrvStorageVolCreateXML volCreateXML; virDrvStorageVolCreateXMLFrom volCreateXMLFrom; virDrvStorageVolDelete volDelete; +virDrvStorageVolWipe volWipe; virDrvStorageVolGetInfo volGetInfo; virDrvStorageVolGetXMLDesc volGetXMLDesc; virDrvStorageVolGetPath volGetPath; -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] Add public API for volume wiping
--- include/libvirt/libvirt.h.in |2 ++ src/libvirt_public.syms |5 + 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0d1b5b5..1430a95 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1224,6 +1224,8 @@ virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr pool, unsigned int flags); int virStorageVolDelete (virStorageVolPtr vol, unsigned int flags); +int virStorageVolWipe (virStorageVolPtr vol, + unsigned int flags); int virStorageVolRef(virStorageVolPtr vol); int virStorageVolFree (virStorageVolPtr vol); diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 64e7505..8c72ec6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -358,4 +358,9 @@ LIBVIRT_0.7.7 { virDomainAbortJob; } LIBVIRT_0.7.5; +LIBVIRT_0.7.8 { +global: + virStorageVolWipe; +} LIBVIRT_0.7.7; + # define new API here using predicted next version number -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Rebased volume wiping API patches
Nobody ack'd (because of general lack of time, I hope) what I think is the final version of the volume wiping API patches, so here is the set again, rebased to the current head. I don't think there is anything controversial in here, as they incorporate all the feedback on previous versions. Dave David Allan (9): Add public API for volume wiping Define the internal driver API for vol wiping Add vol wiping to ESX storage driver struct Implement the public API for vol wiping Define wire protocol format for vol wiping Implement RPC client for vol wiping Implement the remote dispatch bits of vol wiping Simplified version of volume wiping based on feedback from the list. Virsh support for vol wiping daemon/remote.c | 32 + daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 ++ daemon/remote_dispatch_table.h |5 + include/libvirt/libvirt.h.in|2 + src/driver.h|5 + src/esx/esx_storage_driver.c|1 + src/libvirt.c | 47 src/libvirt_public.syms |5 + src/remote/remote_driver.c | 27 src/remote/remote_protocol.c| 11 ++ src/remote/remote_protocol.h|9 ++ src/remote/remote_protocol.x|8 +- src/storage/storage_driver.c| 224 +++ tools/virsh.c | 42 +++ 15 files changed, 426 insertions(+), 1 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] Implement RPC client for vol wiping
--- src/remote/remote_driver.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 11513bd..0ee038e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5538,6 +5538,32 @@ done: } static int +remoteStorageVolWipe(virStorageVolPtr vol, + unsigned int flags) +{ +int rv = -1; +remote_storage_vol_wipe_args args; +struct private_data *priv = vol->conn->storagePrivateData; + +remoteDriverLock(priv); + +make_nonnull_storage_vol(&args.vol, vol); +args.flags = flags; + +if (call(vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_WIPE, + (xdrproc_t) xdr_remote_storage_vol_wipe_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) +goto done; + +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + + +static int remoteStorageVolGetInfo (virStorageVolPtr vol, virStorageVolInfoPtr info) { int rv = -1; @@ -9202,6 +9228,7 @@ static virStorageDriver storage_driver = { .volCreateXML = remoteStorageVolCreateXML, .volCreateXMLFrom = remoteStorageVolCreateXMLFrom, .volDelete = remoteStorageVolDelete, +.volWipe = remoteStorageVolWipe, .volGetInfo = remoteStorageVolGetInfo, .volGetXMLDesc = remoteStorageVolDumpXML, .volGetPath = remoteStorageVolGetPath, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] Define wire protocol format for vol wiping
--- src/remote/remote_protocol.c | 11 +++ src/remote/remote_protocol.h |9 + src/remote/remote_protocol.x |8 +++- 3 files changed, 27 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 701acab..6bd9019 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -2286,6 +2286,17 @@ xdr_remote_storage_vol_delete_args (XDR *xdrs, remote_storage_vol_delete_args *o } bool_t +xdr_remote_storage_vol_wipe_args (XDR *xdrs, remote_storage_vol_wipe_args *objp) +{ + + if (!xdr_remote_nonnull_storage_vol (xdrs, &objp->vol)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; +return TRUE; +} + +bool_t xdr_remote_storage_vol_dump_xml_args (XDR *xdrs, remote_storage_vol_dump_xml_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index f76e6e5..c492ffd 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -1295,6 +1295,12 @@ struct remote_storage_vol_delete_args { }; typedef struct remote_storage_vol_delete_args remote_storage_vol_delete_args; +struct remote_storage_vol_wipe_args { +remote_nonnull_storage_vol vol; +u_int flags; +}; +typedef struct remote_storage_vol_wipe_args remote_storage_vol_wipe_args; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; u_int flags; @@ -1872,6 +1878,7 @@ enum remote_procedure { REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_WIPE = 165, }; typedef enum remote_procedure remote_procedure; @@ -2110,6 +2117,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (XDR *, remote_storage_vol_ extern bool_t xdr_remote_storage_vol_create_xml_from_args (XDR *, remote_storage_vol_create_xml_from_args*); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (XDR *, remote_storage_vol_create_xml_from_ret*); extern bool_t xdr_remote_storage_vol_delete_args (XDR *, remote_storage_vol_delete_args*); +extern bool_t xdr_remote_storage_vol_wipe_args (XDR *, remote_storage_vol_wipe_args*); extern bool_t xdr_remote_storage_vol_dump_xml_args (XDR *, remote_storage_vol_dump_xml_args*); extern bool_t xdr_remote_storage_vol_dump_xml_ret (XDR *, remote_storage_vol_dump_xml_ret*); extern bool_t xdr_remote_storage_vol_get_info_args (XDR *, remote_storage_vol_get_info_args*); @@ -2393,6 +2401,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (); extern bool_t xdr_remote_storage_vol_create_xml_from_args (); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (); extern bool_t xdr_remote_storage_vol_delete_args (); +extern bool_t xdr_remote_storage_vol_wipe_args (); extern bool_t xdr_remote_storage_vol_dump_xml_args (); extern bool_t xdr_remote_storage_vol_dump_xml_ret (); extern bool_t xdr_remote_storage_vol_get_info_args (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5e33da5..868a2c2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1169,6 +1169,11 @@ struct remote_storage_vol_delete_args { unsigned flags; }; +struct remote_storage_vol_wipe_args { +remote_nonnull_storage_vol vol; +unsigned flags; +}; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; unsigned flags; @@ -1703,7 +1708,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DETACH_DEVICE_FLAGS = 161, REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, -REMOTE_PROC_DOMAIN_ABORT_JOB = 164 +REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_WIPE = 165 /* * Notice how the entries are grouped in sets of 10 ? -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] contributor guidelines
Here's a patch to consolidate the contributor guidelines into one place on the website and add a section on the use of goto. There was a small amount of content that was in HACKING but not in hacking.html.in, which I added. I also added Dan's suggested goto labels for common cases to the section on goto, and I removed the contents of HACKING and replaced them with a link to the website. Dave David Allan (1): Consolidating contributor guidelines HACKING | 395 +- docs/hacking.html.in | 125 - 2 files changed, 125 insertions(+), 395 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Consolidating contributor guidelines
* Added a section on the appropriate and inappropriate uses of goto to the HACKING document and website. * Added a few sections that were part of HACKING but not in hacking.html * Removed contents of HACKING file and replaced them with a link to the libvirt.org hacking page. --- HACKING | 395 +- docs/hacking.html.in | 125 - 2 files changed, 125 insertions(+), 395 deletions(-) diff --git a/HACKING b/HACKING index be8725d..e892b60 100644 --- a/HACKING +++ b/HACKING @@ -1,394 +1 @@ -Libvirt contributor guidelines -== - - -General tips for contributing patches -= - -(1) Discuss any large changes on the mailing list first. Post patches -early and listen to feedback. - -(2) Post patches in unified diff format. A command similar to this -should work: - - diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch - -or: - - git diff > libvirt-myfeature.patch - -(3) Split large changes into a series of smaller patches, self-contained -if possible, with an explanation of each patch and an explanation of how -the sequence of patches fits together. - -(4) Make sure your patches apply against libvirt GIT. Developers -only follow GIT and don't care much about released versions. - -(5) Run the automated tests on your code before submitting any changes. -In particular, configure with compile warnings set to -Werror: - - ./configure --enable-compile-warnings=error - -and run the tests: - - make check - make syntax-check - make -C tests valgrind - -The latter test checks for memory leaks. - -If you encounter any failing tests, the VIR_TEST_DEBUG environment variable -may provide extra information to debug the failures. Larger values of -VIR_TEST_DEBUG may provide larger amounts of information: - - VIR_TEST_DEBUG=1 make check(or) - VIR_TEST_DEBUG=2 make check - -Also, individual tests can be run from inside the 'tests/' directory, like: - - ./qemuxml2xmltest - -(6) Update tests and/or documentation, particularly if you are adding -a new feature or changing the output of a program. - - -There is more on this subject, including lots of links to background -reading on the subject, on this page: - - http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/ - - - -Code indentation - -Libvirt's C source code generally adheres to some basic code-formatting -conventions. The existing code base is not totally consistent on this -front, but we do prefer that contributed code be formatted similarly. -In short, use spaces-not-TABs for indentation, use 4 spaces for each -indentation level, and other than that, follow the K&R style. - -If you use Emacs, add the following to one of one of your start-up files -(e.g., ~/.emacs), to help ensure that you get indentation right: - - ;;; When editing C sources in libvirt, use this style. - (defun libvirt-c-mode () -"C mode with adjusted defaults for use with libvirt." -(interactive) -(c-set-style "K&R") -(setq indent-tabs-mode nil) ; indent using spaces, not TABs -(setq c-indent-level 4) -(setq c-basic-offset 4)) - (add-hook 'c-mode-hook - '(lambda () (if (string-match "/libvirt" (buffer-file-name)) - (libvirt-c-mode - -Code formatting (especially for new code) -= -With new code, we can be even more strict. -Please apply the following function (using GNU indent) to any new code. -Note that this also gives you an idea of the type of spacing we prefer -around operators and keywords: - - indent-libvirt() - { -indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \ - -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \ - --no-tabs "$@" - } - -Note that sometimes you'll have to postprocess that output further, by -piping it through "expand -i", since some leading TABs can get through. -Usually they're in macro definitions or strings, and should be converted -anyhow. - - -C types -=== -Use the right type. - -Scalars -If you're using "int" or "long", odds are good that there's a better type. -If a variable is counting something, be sure to declare it with an -unsigned type. -If it's memory-size-related, use size_t (use ssize_t only if required). -If it's file-size related, use uintmax_t, or maybe off_t. -If it's file-offset related (i.e., signed), use off_t. -If it's just counting small numbers use "unsigned int"; -(on all but oddball embedded systems, you can assume that that -type is at least four bytes wide). -If a variable has boolean semantics, give it the "bool" type -and use the corresponding "true" and "false" macros. It's ok -to include , since libvirt's use of gnulib ensures -that it exists and is usable. -In the unusual event that you require a specific width, use a -standard type like int32_t, uin
[libvirt] [PATCH 0/1] Documenting the uses of goto
I added a section on good and bad uses of goto to the hacking document and website. Also, should we dispense with the HACKING file's content altogether and replace it with a link to hacking.html on the website? It seems like unnecessary work to maintain identical content in both places. Dave David Allan (1): The use of goto HACKING | 32 docs/hacking.html.in | 41 + 2 files changed, 73 insertions(+), 0 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] The use of goto
* Added a section on the appropriate and inappropriate uses of goto to the HACKING document and website. --- HACKING | 32 docs/hacking.html.in | 41 + 2 files changed, 73 insertions(+), 0 deletions(-) diff --git a/HACKING b/HACKING index be8725d..db99630 100644 --- a/HACKING +++ b/HACKING @@ -362,6 +362,38 @@ their jobs and cross-check format strings with the number and types of arguments. +Use of goto +=== + +The use of goto is not forbidden, and goto is widely used throughout +libvirt. While the uncontrolled use of goto will quickly lead to +unmaintainable code, there is a place for it in well structured code +where its use increases readability and maintainability. + +The typical use of goto is to jump to cleanup code in the case of a +long list of actions, any of which may fail and cause the entire +operation to fail. In this case, a function will have a single label +at the end of the funcion. It's almost always ok to use this style. +In particular, if the cleanup code only involves free'ing memory, then +having multiple labels is overkill. VIR_FREE() and every function +named XXXFree() in libvirt is required to handle NULL as its arg. +Thus you can safely call free on all the variables even if they were +not yet allocated (yes they have to have been initialized to NULL). +This is much simpler and clearer than having multiple labels. + +There are a couple of signs that a particular use of goto is not ok. +The first is the use of multiple labels. If you find yourself using +multiple labels, you're strongly encouraged to rework your code to +eliminate all but one of them. The second is jumping back up, above +the current line of code being executed. Please use some combination +of looping constructs to re-execute code instead; it's almost +certainly going to be more understandable by others. + +Although libvirt does not encourage the Linux kernel wind/unwind style +of multiple labels, there's a good general discussion of the issue at: + +http://kerneltrap.org/node/553/2131 + Libvirt commiters guidelines diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 788a49b..e10d2dd 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -389,6 +389,47 @@ of arguments. +Use of goto + + + The use of goto is not forbidden, and goto is widely used + throughout libvirt. While the uncontrolled use of goto will + quickly lead to unmaintainable code, there is a place for it in + well structured code where its use increases readability and + maintainability. + + + + The typical use of goto is to jump to cleanup code in the case + of a long list of actions, any of which may fail and cause the + entire operation to fail. In this case, a function will have a + single label at the end of the funcion. It's almost always ok + to use this style. In particular, if the cleanup code only + involves free'ing memory, then having multiple labels is + overkill. VIR_FREE() and every function named XXXFree() in + libvirt is required to handle NULL as its arg. Thus you can + safely call free on all the variables even if they were not yet + allocated (yes they have to have been initialized to NULL). + This is much simpler and clearer than having multiple labels. + + + + There are a couple of signs that a particular use of goto is not + ok. The first is the use of multiple labels. If you find + yourself using multiple labels, you're strongly encouraged to + rework your code to eliminate all but one of them. The second + is jumping back up, above the current line of code being + executed. Please use some combination of looping constructs to + re-execute code instead; it's almost certainly going to be more + understandable by others. + + + + Although libvirt does not encourage the Linux kernel wind/unwind + style of multiple labels, there's a good general discussion of + the issue archived at + http://kerneltrap.org/node/553/2131>KernelTrap + Libvirt commiters guidelines -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] Define wire protocol format for vol wiping
--- src/remote/remote_protocol.c | 11 +++ src/remote/remote_protocol.h |9 + src/remote/remote_protocol.x |8 +++- 3 files changed, 27 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 701acab..6bd9019 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -2286,6 +2286,17 @@ xdr_remote_storage_vol_delete_args (XDR *xdrs, remote_storage_vol_delete_args *o } bool_t +xdr_remote_storage_vol_wipe_args (XDR *xdrs, remote_storage_vol_wipe_args *objp) +{ + + if (!xdr_remote_nonnull_storage_vol (xdrs, &objp->vol)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; +return TRUE; +} + +bool_t xdr_remote_storage_vol_dump_xml_args (XDR *xdrs, remote_storage_vol_dump_xml_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index e06d73f..1b606e9 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -1295,6 +1295,12 @@ struct remote_storage_vol_delete_args { }; typedef struct remote_storage_vol_delete_args remote_storage_vol_delete_args; +struct remote_storage_vol_wipe_args { +remote_nonnull_storage_vol vol; +u_int flags; +}; +typedef struct remote_storage_vol_wipe_args remote_storage_vol_wipe_args; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; u_int flags; @@ -1872,6 +1878,7 @@ enum remote_procedure { REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_WIPE = 165, }; typedef enum remote_procedure remote_procedure; @@ -2110,6 +2117,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (XDR *, remote_storage_vol_ extern bool_t xdr_remote_storage_vol_create_xml_from_args (XDR *, remote_storage_vol_create_xml_from_args*); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (XDR *, remote_storage_vol_create_xml_from_ret*); extern bool_t xdr_remote_storage_vol_delete_args (XDR *, remote_storage_vol_delete_args*); +extern bool_t xdr_remote_storage_vol_wipe_args (XDR *, remote_storage_vol_wipe_args*); extern bool_t xdr_remote_storage_vol_dump_xml_args (XDR *, remote_storage_vol_dump_xml_args*); extern bool_t xdr_remote_storage_vol_dump_xml_ret (XDR *, remote_storage_vol_dump_xml_ret*); extern bool_t xdr_remote_storage_vol_get_info_args (XDR *, remote_storage_vol_get_info_args*); @@ -2393,6 +2401,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (); extern bool_t xdr_remote_storage_vol_create_xml_from_args (); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (); extern bool_t xdr_remote_storage_vol_delete_args (); +extern bool_t xdr_remote_storage_vol_wipe_args (); extern bool_t xdr_remote_storage_vol_dump_xml_args (); extern bool_t xdr_remote_storage_vol_dump_xml_ret (); extern bool_t xdr_remote_storage_vol_get_info_args (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5e33da5..868a2c2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1169,6 +1169,11 @@ struct remote_storage_vol_delete_args { unsigned flags; }; +struct remote_storage_vol_wipe_args { +remote_nonnull_storage_vol vol; +unsigned flags; +}; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; unsigned flags; @@ -1703,7 +1708,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DETACH_DEVICE_FLAGS = 161, REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, -REMOTE_PROC_DOMAIN_ABORT_JOB = 164 +REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_WIPE = 165 /* * Notice how the entries are grouped in sets of 10 ? -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] Virsh support for vol wiping
--- tools/virsh.c | 42 ++ 1 files changed, 42 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 65487ed..00f412e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5221,6 +5221,47 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd) /* + * "vol-wipe" command + */ +static const vshCmdInfo info_vol_wipe[] = { +{"help", gettext_noop("wipe a vol")}, +{"desc", gettext_noop("Ensure data previously on a volume is not accessible to future reads")}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_wipe[] = { +{"pool", VSH_OT_STRING, 0, gettext_noop("pool name or uuid")}, +{"vol", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("vol name, key or path")}, +{NULL, 0, 0, NULL} +}; + +static int +cmdVolWipe(vshControl *ctl, const vshCmd *cmd) +{ +virStorageVolPtr vol; +int ret = TRUE; +char *name; + +if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) +return FALSE; + +if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) { +return FALSE; +} + +if (virStorageVolWipe(vol, 0) == 0) { +vshPrint(ctl, _("Vol %s wiped\n"), name); +} else { +vshError(ctl, _("Failed to wipe vol %s"), name); +ret = FALSE; +} + +virStorageVolFree(vol); +return ret; +} + + +/* * "vol-info" command */ static const vshCmdInfo info_vol_info[] = { @@ -7758,6 +7799,7 @@ static const vshCmdDef commands[] = { {"vol-create-as", cmdVolCreateAs, opts_vol_create_as, info_vol_create_as}, {"vol-clone", cmdVolClone, opts_vol_clone, info_vol_clone}, {"vol-delete", cmdVolDelete, opts_vol_delete, info_vol_delete}, +{"vol-wipe", cmdVolWipe, opts_vol_wipe, info_vol_wipe}, {"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml}, {"vol-info", cmdVolInfo, opts_vol_info, info_vol_info}, {"vol-list", cmdVolList, opts_vol_list, info_vol_list}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] Implement the remote dispatch bits of vol wiping
* I had to remove daemon/remote_dispatch* and do a make remote.c in daemon in order to get the dispatch files regenerated properly. The make was failing before I did that. --- daemon/remote.c | 32 daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 daemon/remote_dispatch_table.h |5 + 4 files changed, 46 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d4713b2..9548169 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4273,6 +4273,38 @@ remoteDispatchStorageVolDelete (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchStorageVolWipe(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_storage_vol_wipe_args *args, + void *ret ATTRIBUTE_UNUSED) +{ +int retval = -1; +virStorageVolPtr vol; + +vol = get_nonnull_storage_vol(conn, args->vol); +if (vol == NULL) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +if (virStorageVolWipe(vol, args->flags) == -1) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +retval = 0; + +out: +if (vol != NULL) { +virStorageVolFree(vol); +} +return retval; +} + +static int remoteDispatchStorageVolGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index f97155b..32a6d78 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -140,3 +140,4 @@ remote_cpu_baseline_args val_remote_cpu_baseline_args; remote_domain_get_job_info_args val_remote_domain_get_job_info_args; remote_domain_abort_job_args val_remote_domain_abort_job_args; +remote_storage_vol_wipe_args val_remote_storage_vol_wipe_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index b81c8c3..4c944f6 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -1298,6 +1298,14 @@ static int remoteDispatchStorageVolLookupByPath( remote_error *err, remote_storage_vol_lookup_by_path_args *args, remote_storage_vol_lookup_by_path_ret *ret); +static int remoteDispatchStorageVolWipe( +struct qemud_server *server, +struct qemud_client *client, +virConnectPtr conn, +remote_message_header *hdr, +remote_error *err, +remote_storage_vol_wipe_args *args, +void *ret); static int remoteDispatchSupportsFeature( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index 5ad6bff..28e7d9a 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -827,3 +827,8 @@ .args_filter = (xdrproc_t) xdr_remote_domain_abort_job_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* StorageVolWipe => 165 */ +.fn = (dispatch_fn) remoteDispatchStorageVolWipe, +.args_filter = (xdrproc_t) xdr_remote_storage_vol_wipe_args, +.ret_filter = (xdrproc_t) xdr_void, +}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] Simplified version of volume wiping based on feedback from the list.
--- src/storage/storage_driver.c | 224 ++ 1 files changed, 224 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f3be6de..b0afb05 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include + #if HAVE_PWD_H #include #endif @@ -1518,6 +1521,226 @@ cleanup: return ret; } + +/* If the volume we're wiping is already a sparse file, we simply + * truncate and extend it to its original size, filling it with + * zeroes. This behavior is guaranteed by POSIX: + * + * http://www.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html + * + * If fildes refers to a regular file, the ftruncate() function shall + * cause the size of the file to be truncated to length. If the size + * of the file previously exceeded length, the extra data shall no + * longer be available to reads on the file. If the file previously + * was smaller than this size, ftruncate() shall increase the size of + * the file. If the file size is increased, the extended area shall + * appear as if it were zero-filled. + */ +static int +storageVolumeZeroSparseFile(virStorageVolDefPtr vol, +off_t size, +int fd) +{ +int ret = -1; +char errbuf[64]; + +ret = ftruncate(fd, 0); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to 0 bytes: '%s'"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +ret = ftruncate(fd, size); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to %ju bytes: '%s'\n"), + vol->target.path, (intmax_t)size, + virStrerror(errno, errbuf, sizeof(errbuf))); +} + +out: +return ret; +} + + +static int +storageWipeExtent(virStorageVolDefPtr vol, + int fd, + off_t extent_start, + off_t extent_length, + char *writebuf, + size_t writebuf_length, + size_t *bytes_wiped) +{ +int ret = -1, written = 0; +off_t remaining = 0; +size_t write_size = 0; +char errbuf[64]; + +VIR_DEBUG("extent logical start: %ju len: %ju", + (intmax_t)extent_start, (intmax_t)extent_length); + +if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) { +virReportSystemError(ret, + _("Failed to seek to position %ju in volume " + "with path '%s': '%s'"), + (intmax_t)extent_start, vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +remaining = extent_length; +while (remaining > 0) { + +write_size = (writebuf_length < remaining) ? writebuf_length : remaining; +written = safewrite(fd, writebuf, write_size); +if (written < 0) { +virReportSystemError(written, + _("Failed to write to storage volume with " + "path '%s': '%s' " + "(attempted to write %zu bytes)"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf)), + write_size); +goto out; +} + +*bytes_wiped += written; +remaining -= written; +} + +VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", + *bytes_wiped, vol->target.path); + +ret = 0; + +out: +return ret; +} + + +static int +storageVolumeWipeInternal(virStorageVolDefPtr def) +{ +int ret = -1, fd = -1; +char errbuf[64]; +struct stat st; +char *writebuf = NULL; +size_t bytes_wiped = 0; + +VIR_DEBUG("Wiping volume with path '%s'", def->target.path); + +fd = open(def->target.path, O_RDWR); +if (fd == -1) { +VIR_ERROR("Failed to open storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (fstat(fd, &st) == -1) { +VIR_ERROR("Failed to stat storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { +ret = storageVolumeZeroSparseFile(def, st.st_size, fd); +} else { + +if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { +virReportOOME
[libvirt] [PATCH 4/9] Implement the public API for vol wiping
--- src/libvirt.c | 47 +++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index d9242bc..6abf74f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8468,6 +8468,53 @@ error: /** + * virStorageVolWipe: + * @vol: pointer to storage volume + * @flags: future flags, use 0 for now + * + * Ensure data previously on a volume is not accessible to future reads + * + * Returns 0 on success, or -1 on error + */ +int +virStorageVolWipe(virStorageVolPtr vol, + unsigned int flags) +{ +virConnectPtr conn; +VIR_DEBUG("vol=%p, flags=%u", vol, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { +virLibStorageVolError(NULL, VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); +virDispatchError(NULL); +return (-1); +} + +conn = vol->conn; +if (conn->flags & VIR_CONNECT_RO) { +virLibStorageVolError(vol, VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (conn->storageDriver && conn->storageDriver->volWipe) { +int ret; +ret = conn->storageDriver->volWipe(vol, flags); +if (ret < 0) { +goto error; +} +return ret; +} + +virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(vol->conn); +return -1; +} + + +/** * virStorageVolFree: * @vol: pointer to storage volume * -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] Implement RPC client for vol wiping
--- src/remote/remote_driver.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b7b2e09..9d28c84 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5538,6 +5538,32 @@ done: } static int +remoteStorageVolWipe(virStorageVolPtr vol, + unsigned int flags) +{ +int rv = -1; +remote_storage_vol_wipe_args args; +struct private_data *priv = vol->conn->storagePrivateData; + +remoteDriverLock(priv); + +make_nonnull_storage_vol(&args.vol, vol); +args.flags = flags; + +if (call(vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_WIPE, + (xdrproc_t) xdr_remote_storage_vol_wipe_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) +goto done; + +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + + +static int remoteStorageVolGetInfo (virStorageVolPtr vol, virStorageVolInfoPtr info) { int rv = -1; @@ -9202,6 +9228,7 @@ static virStorageDriver storage_driver = { .volCreateXML = remoteStorageVolCreateXML, .volCreateXMLFrom = remoteStorageVolCreateXMLFrom, .volDelete = remoteStorageVolDelete, +.volWipe = remoteStorageVolWipe, .volGetInfo = remoteStorageVolGetInfo, .volGetXMLDesc = remoteStorageVolDumpXML, .volGetPath = remoteStorageVolGetPath, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/9] Add vol wiping to ESX storage driver struct
--- src/esx/esx_storage_driver.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index d09831a..3062473 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -101,6 +101,7 @@ static virStorageDriver esxStorageDriver = { NULL, /* volCreateXML */ NULL, /* volCreateXMLFrom */ NULL, /* volDelete */ +NULL, /* volWipe */ NULL, /* volGetInfo */ NULL, /* volGetXMLDesc */ NULL, /* volGetPath */ -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] Add public API for volume wiping
--- include/libvirt/libvirt.h.in |2 ++ src/libvirt_public.syms |1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0d1b5b5..1430a95 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1224,6 +1224,8 @@ virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr pool, unsigned int flags); int virStorageVolDelete (virStorageVolPtr vol, unsigned int flags); +int virStorageVolWipe (virStorageVolPtr vol, + unsigned int flags); int virStorageVolRef(virStorageVolPtr vol); int virStorageVolFree (virStorageVolPtr vol); diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 64e7505..ebf13e6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -356,6 +356,7 @@ LIBVIRT_0.7.7 { virConnectBaselineCPU; virDomainGetJobInfo; virDomainAbortJob; + virStorageVolWipe; } LIBVIRT_0.7.5; # define new API here using predicted next version number -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] Define the internal driver API for vol wiping
--- src/driver.h |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index 7b7dfea..6e1403a 100644 --- a/src/driver.h +++ b/src/driver.h @@ -722,6 +722,10 @@ typedef int unsigned int flags); typedef int +(*virDrvStorageVolWipe) (virStorageVolPtr vol, + unsigned int flags); + +typedef int (*virDrvStorageVolGetInfo) (virStorageVolPtr vol, virStorageVolInfoPtr info); typedef char * @@ -790,6 +794,7 @@ struct _virStorageDriver { virDrvStorageVolCreateXML volCreateXML; virDrvStorageVolCreateXMLFrom volCreateXMLFrom; virDrvStorageVolDelete volDelete; +virDrvStorageVolWipe volWipe; virDrvStorageVolGetInfo volGetInfo; virDrvStorageVolGetXMLDesc volGetXMLDesc; virDrvStorageVolGetPath volGetPath; -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Storage volume wiping API
Here's the revised volume wiping API patchset (thanks to Dan for the improved name). I'm sending a replacement patchset since the name change made for a very large and messy incremental. This set contains everybody's feedback, which was all very helpful. Many thanks. Dave David Allan (9): Add public API for volume wiping Define the internal driver API for vol wiping Add vol wiping to ESX storage driver struct Implement the public API for vol wiping Define wire protocol format for vol wiping Implement RPC client for vol wiping Implement the remote dispatch bits of vol wiping Simplified version of volume wiping based on feedback from the list. Virsh support for vol wiping daemon/remote.c | 32 + daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 ++ daemon/remote_dispatch_table.h |5 + include/libvirt/libvirt.h.in|2 + src/driver.h|5 + src/esx/esx_storage_driver.c|1 + src/libvirt.c | 47 src/libvirt_public.syms |1 + src/remote/remote_driver.c | 27 src/remote/remote_protocol.c| 11 ++ src/remote/remote_protocol.h|9 ++ src/remote/remote_protocol.x|8 +- src/storage/storage_driver.c| 224 +++ tools/virsh.c | 42 +++ 15 files changed, 422 insertions(+), 1 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] Virsh support for vol zeroing
--- tools/virsh.c | 42 ++ 1 files changed, 42 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a6a637d..e52b011 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5221,6 +5221,47 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd) /* + * "vol-zero-out" command + */ +static const vshCmdInfo info_vol_zero_out[] = { +{"help", gettext_noop("zero out a vol")}, +{"desc", gettext_noop("Ensure further reads from a volume return zeroes.")}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_zero_out[] = { +{"pool", VSH_OT_STRING, 0, gettext_noop("pool name or uuid")}, +{"vol", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("vol name, key or path")}, +{NULL, 0, 0, NULL} +}; + +static int +cmdVolZeroOut(vshControl *ctl, const vshCmd *cmd) +{ +virStorageVolPtr vol; +int ret = TRUE; +char *name; + +if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) +return FALSE; + +if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) { +return FALSE; +} + +if (virStorageVolZeroOut(vol, 0) == 0) { +vshPrint(ctl, _("Vol %s zeroed out\n"), name); +} else { +vshError(ctl, _("Failed to zero out vol %s"), name); +ret = FALSE; +} + +virStorageVolFree(vol); +return ret; +} + + +/* * "vol-info" command */ static const vshCmdInfo info_vol_info[] = { @@ -7758,6 +7799,7 @@ static const vshCmdDef commands[] = { {"vol-create-as", cmdVolCreateAs, opts_vol_create_as, info_vol_create_as}, {"vol-clone", cmdVolClone, opts_vol_clone, info_vol_clone}, {"vol-delete", cmdVolDelete, opts_vol_delete, info_vol_delete}, +{"vol-zero-out", cmdVolZeroOut, opts_vol_zero_out, info_vol_zero_out}, {"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml}, {"vol-info", cmdVolInfo, opts_vol_info, info_vol_info}, {"vol-list", cmdVolList, opts_vol_list, info_vol_list}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] Implement the remote dispatch bits of vol zeroing
* I had to remove daemon/remote_dispatch* and do a make remote.c in daemon in order to get the dispatch files regenerated properly. The make was failing before I did that. --- daemon/remote.c | 32 daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 daemon/remote_dispatch_table.h |5 + 4 files changed, 46 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d4713b2..7621044 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4273,6 +4273,38 @@ remoteDispatchStorageVolDelete (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchStorageVolZeroOut(struct qemud_server *server ATTRIBUTE_UNUSED, +struct qemud_client *client ATTRIBUTE_UNUSED, +virConnectPtr conn, +remote_message_header *hdr ATTRIBUTE_UNUSED, +remote_error *rerr, +remote_storage_vol_zero_out_args *args, +void *ret ATTRIBUTE_UNUSED) +{ +int retval = -1; +virStorageVolPtr vol; + +vol = get_nonnull_storage_vol(conn, args->vol); +if (vol == NULL) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +if (virStorageVolZeroOut(vol, args->flags) == -1) { +remoteDispatchConnError(rerr, conn); +goto out; +} + +retval = 0; + +out: +if (vol != NULL) { +virStorageVolFree(vol); +} +return retval; +} + +static int remoteDispatchStorageVolGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index f97155b..3ddbec4 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -140,3 +140,4 @@ remote_cpu_baseline_args val_remote_cpu_baseline_args; remote_domain_get_job_info_args val_remote_domain_get_job_info_args; remote_domain_abort_job_args val_remote_domain_abort_job_args; +remote_storage_vol_zero_out_args val_remote_storage_vol_zero_out_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index b81c8c3..db8e323 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -1298,6 +1298,14 @@ static int remoteDispatchStorageVolLookupByPath( remote_error *err, remote_storage_vol_lookup_by_path_args *args, remote_storage_vol_lookup_by_path_ret *ret); +static int remoteDispatchStorageVolZeroOut( +struct qemud_server *server, +struct qemud_client *client, +virConnectPtr conn, +remote_message_header *hdr, +remote_error *err, +remote_storage_vol_zero_out_args *args, +void *ret); static int remoteDispatchSupportsFeature( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index 5ad6bff..11d9744 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -827,3 +827,8 @@ .args_filter = (xdrproc_t) xdr_remote_domain_abort_job_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* StorageVolZeroOut => 165 */ +.fn = (dispatch_fn) remoteDispatchStorageVolZeroOut, +.args_filter = (xdrproc_t) xdr_remote_storage_vol_zero_out_args, +.ret_filter = (xdrproc_t) xdr_void, +}, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] Implement RPC client for vol zeroing
--- src/remote/remote_driver.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b7b2e09..739f6bb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5538,6 +5538,32 @@ done: } static int +remoteStorageVolZeroOut(virStorageVolPtr vol, +unsigned int flags) +{ +int rv = -1; +remote_storage_vol_zero_out_args args; +struct private_data *priv = vol->conn->storagePrivateData; + +remoteDriverLock(priv); + +make_nonnull_storage_vol(&args.vol, vol); +args.flags = flags; + +if (call(vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_ZERO_OUT, + (xdrproc_t) xdr_remote_storage_vol_zero_out_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) +goto done; + +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + + +static int remoteStorageVolGetInfo (virStorageVolPtr vol, virStorageVolInfoPtr info) { int rv = -1; @@ -9202,6 +9228,7 @@ static virStorageDriver storage_driver = { .volCreateXML = remoteStorageVolCreateXML, .volCreateXMLFrom = remoteStorageVolCreateXMLFrom, .volDelete = remoteStorageVolDelete, +.volZeroOut = remoteStorageVolZeroOut, .volGetInfo = remoteStorageVolGetInfo, .volGetXMLDesc = remoteStorageVolDumpXML, .volGetPath = remoteStorageVolGetPath, -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] Add public API for volume zeroing
--- include/libvirt/libvirt.h.in |2 ++ src/libvirt_public.syms |1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0d1b5b5..6f755c5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1224,6 +1224,8 @@ virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr pool, unsigned int flags); int virStorageVolDelete (virStorageVolPtr vol, unsigned int flags); +int virStorageVolZeroOut(virStorageVolPtr vol, + unsigned int flags); int virStorageVolRef(virStorageVolPtr vol); int virStorageVolFree (virStorageVolPtr vol); diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 64e7505..260cbc1 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -356,6 +356,7 @@ LIBVIRT_0.7.7 { virConnectBaselineCPU; virDomainGetJobInfo; virDomainAbortJob; + virStorageVolZeroOut; } LIBVIRT_0.7.5; # define new API here using predicted next version number -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] Simplified version of volume zeroing based on feedback from the list.
--- src/storage/storage_driver.c | 218 ++ 1 files changed, 218 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6b1045a..9e63689 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include + #if HAVE_PWD_H #include #endif @@ -1518,6 +1521,220 @@ cleanup: return ret; } + +/* If the volume we're zeroing is already a sparse file, we simply + * truncate and extend it to its original size, filling it with + * zeroes. This behavior is guaranteed by POSIX: + * + * http://www.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html + * + * If fildes refers to a regular file, the ftruncate() function shall + * cause the size of the file to be truncated to length. If the size + * of the file previously exceeded length, the extra data shall no + * longer be available to reads on the file. If the file previously + * was smaller than this size, ftruncate() shall increase the size of + * the file. If the file size is increased, the extended area shall + * appear as if it were zero-filled. + */ +static int +storageVolumeZeroSparseFile(virStorageVolDefPtr vol, +struct stat *st, +int fd) +{ +int ret = -1; +char errbuf[64]; + +ret = ftruncate(fd, 0); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to 0 bytes: '%s'"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +ret = ftruncate(fd, st->st_size); +if (ret == -1) { +virReportSystemError(ret, + _("Failed to truncate volume with " + "path '%s' to %llu bytes: '%s'\n"), + vol->target.path, st->st_size, + virStrerror(errno, errbuf, sizeof(errbuf))); +} + +out: +return ret; +} + + +static int +storageZeroExtent(virStorageVolDefPtr vol, + struct stat *st, + int fd, + size_t extent_start, + size_t extent_length, + char *writebuf, + size_t *bytes_zeroed) +{ +int ret = -1, written; +size_t remaining, write_size; +char errbuf[64]; + +VIR_DEBUG("extent logical start: %zu len: %zu ", + extent_start, extent_length); + +if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) { +virReportSystemError(ret, "Failed to seek to position %zu in volume " + "with path '%s': '%s'", + extent_start, vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +remaining = extent_length; +while (remaining > 0) { + +write_size = (st->st_blksize < remaining) ? st->st_blksize : remaining; +written = safewrite(fd, writebuf, write_size); +if (written < 0) { +virReportSystemError(written, + _("Failed to write to storage volume with " + "path '%s': '%s' " + "(attempted to write %d bytes)"), + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf)), + write_size); +goto out; +} + +*bytes_zeroed += written; +remaining -= written; +} + +VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", + *bytes_zeroed, vol->target.path); + +ret = 0; + +out: +return ret; +} + + +static int +storageVolumeZeroOutInternal(virStorageVolDefPtr def) +{ +int ret = -1, fd = -1; +char errbuf[64]; +struct stat st; +char *writebuf; +size_t bytes_zeroed = 0; + +VIR_DEBUG("Zeroing out volume with path '%s'", def->target.path); + +fd = open(def->target.path, O_RDWR); +if (fd == -1) { +VIR_ERROR("Failed to open storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +memset(&st, 0, sizeof(st)); + +if (fstat(fd, &st) == -1) { +VIR_ERROR("Failed to stat storage volume with path '%s': '%s'", + def->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { +ret = storageVolumeZeroSparseFile(def, &st, fd); +} else { + +if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { +virReportOOMError(); +goto out; +} + +
[libvirt] [PATCH 4/9] Implement the public API for vol zeroing
--- src/libvirt.c | 47 +++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index d9242bc..1434874 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8468,6 +8468,53 @@ error: /** + * virStorageVolZeroOut: + * @vol: pointer to storage volume + * @flags: future flags, use 0 for now + * + * Ensure that future reads from the storage volume return zeroes. + * + * Returns 0 on success, or -1 on error + */ +int +virStorageVolZeroOut(virStorageVolPtr vol, + unsigned int flags) +{ +virConnectPtr conn; +VIR_DEBUG("vol=%p, flags=%u", vol, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { +virLibStorageVolError(NULL, VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); +virDispatchError(NULL); +return (-1); +} + +conn = vol->conn; +if (conn->flags & VIR_CONNECT_RO) { +virLibStorageVolError(vol, VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (conn->storageDriver && conn->storageDriver->volZeroOut) { +int ret; +ret = conn->storageDriver->volZeroOut(vol, flags); +if (ret < 0) { +goto error; +} +return ret; +} + +virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(vol->conn); +return -1; +} + + +/** * virStorageVolFree: * @vol: pointer to storage volume * -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/9] Define the internal driver API for vol zeroing
--- src/driver.h |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index 7b7dfea..f7b41e9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -722,6 +722,10 @@ typedef int unsigned int flags); typedef int +(*virDrvStorageVolZeroOut) (virStorageVolPtr vol, + unsigned int flags); + +typedef int (*virDrvStorageVolGetInfo) (virStorageVolPtr vol, virStorageVolInfoPtr info); typedef char * @@ -790,6 +794,7 @@ struct _virStorageDriver { virDrvStorageVolCreateXML volCreateXML; virDrvStorageVolCreateXMLFrom volCreateXMLFrom; virDrvStorageVolDelete volDelete; +virDrvStorageVolZeroOut volZeroOut; virDrvStorageVolGetInfo volGetInfo; virDrvStorageVolGetXMLDesc volGetXMLDesc; virDrvStorageVolGetPath volGetPath; -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] Define wire protocol format for vol zeroing
--- src/remote/remote_protocol.c | 11 +++ src/remote/remote_protocol.h |9 + src/remote/remote_protocol.x |8 +++- 3 files changed, 27 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 701acab..bd52be0 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -2286,6 +2286,17 @@ xdr_remote_storage_vol_delete_args (XDR *xdrs, remote_storage_vol_delete_args *o } bool_t +xdr_remote_storage_vol_zero_out_args (XDR *xdrs, remote_storage_vol_zero_out_args *objp) +{ + + if (!xdr_remote_nonnull_storage_vol (xdrs, &objp->vol)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; +return TRUE; +} + +bool_t xdr_remote_storage_vol_dump_xml_args (XDR *xdrs, remote_storage_vol_dump_xml_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index e06d73f..f98f512 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -1295,6 +1295,12 @@ struct remote_storage_vol_delete_args { }; typedef struct remote_storage_vol_delete_args remote_storage_vol_delete_args; +struct remote_storage_vol_zero_out_args { +remote_nonnull_storage_vol vol; +u_int flags; +}; +typedef struct remote_storage_vol_zero_out_args remote_storage_vol_zero_out_args; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; u_int flags; @@ -1872,6 +1878,7 @@ enum remote_procedure { REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_ZERO_OUT = 165, }; typedef enum remote_procedure remote_procedure; @@ -2110,6 +2117,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (XDR *, remote_storage_vol_ extern bool_t xdr_remote_storage_vol_create_xml_from_args (XDR *, remote_storage_vol_create_xml_from_args*); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (XDR *, remote_storage_vol_create_xml_from_ret*); extern bool_t xdr_remote_storage_vol_delete_args (XDR *, remote_storage_vol_delete_args*); +extern bool_t xdr_remote_storage_vol_zero_out_args (XDR *, remote_storage_vol_zero_out_args*); extern bool_t xdr_remote_storage_vol_dump_xml_args (XDR *, remote_storage_vol_dump_xml_args*); extern bool_t xdr_remote_storage_vol_dump_xml_ret (XDR *, remote_storage_vol_dump_xml_ret*); extern bool_t xdr_remote_storage_vol_get_info_args (XDR *, remote_storage_vol_get_info_args*); @@ -2393,6 +2401,7 @@ extern bool_t xdr_remote_storage_vol_create_xml_ret (); extern bool_t xdr_remote_storage_vol_create_xml_from_args (); extern bool_t xdr_remote_storage_vol_create_xml_from_ret (); extern bool_t xdr_remote_storage_vol_delete_args (); +extern bool_t xdr_remote_storage_vol_zero_out_args (); extern bool_t xdr_remote_storage_vol_dump_xml_args (); extern bool_t xdr_remote_storage_vol_dump_xml_ret (); extern bool_t xdr_remote_storage_vol_get_info_args (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5e33da5..d44da4d 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1169,6 +1169,11 @@ struct remote_storage_vol_delete_args { unsigned flags; }; +struct remote_storage_vol_zero_out_args { +remote_nonnull_storage_vol vol; +unsigned flags; +}; + struct remote_storage_vol_dump_xml_args { remote_nonnull_storage_vol vol; unsigned flags; @@ -1703,7 +1708,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DETACH_DEVICE_FLAGS = 161, REMOTE_PROC_CPU_BASELINE = 162, REMOTE_PROC_DOMAIN_GET_JOB_INFO = 163, -REMOTE_PROC_DOMAIN_ABORT_JOB = 164 +REMOTE_PROC_DOMAIN_ABORT_JOB = 164, +REMOTE_PROC_STORAGE_VOL_ZERO_OUT = 165 /* * Notice how the entries are grouped in sets of 10 ? -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] Fix ESX storage driver struct initializer
--- src/esx/esx_storage_driver.c | 39 +++ 1 files changed, 3 insertions(+), 36 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index d09831a..84f0339 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -70,42 +70,9 @@ esxStorageClose(virConnectPtr conn) static virStorageDriver esxStorageDriver = { -"ESX", /* name */ -esxStorageOpen,/* open */ -esxStorageClose, /* close */ -NULL, /* numOfPools */ -NULL, /* listPools */ -NULL, /* numOfDefinedPools */ -NULL, /* listDefinedPools */ -NULL, /* findPoolSources */ -NULL, /* poolLookupByName */ -NULL, /* poolLookupByUUID */ -NULL, /* poolLookupByVolume */ -NULL, /* poolCreateXML */ -NULL, /* poolDefineXML */ -NULL, /* poolBuild */ -NULL, /* poolUndefine */ -NULL, /* poolCreate */ -NULL, /* poolDestroy */ -NULL, /* poolDelete */ -NULL, /* poolRefresh */ -NULL, /* poolGetInfo */ -NULL, /* poolGetXMLDesc */ -NULL, /* poolGetAutostart */ -NULL, /* poolSetAutostart */ -NULL, /* poolNumOfVolumes */ -NULL, /* poolListVolumes */ -NULL, /* volLookupByName */ -NULL, /* volLookupByKey */ -NULL, /* volLookupByPath */ -NULL, /* volCreateXML */ -NULL, /* volCreateXMLFrom */ -NULL, /* volDelete */ -NULL, /* volGetInfo */ -NULL, /* volGetXMLDesc */ -NULL, /* volGetPath */ -NULL, /* poolIsActive */ -NULL, /* poolIsPersistent */ +.name = "ESX", +.open = esxStorageOpen, +.close = esxStorageClose }; -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] Vol Zeroing API
This patch set contains a new API for zeroing out volumes. I called it zero out rather than overwrite because it doesn't overwrite anything in the case of sparse files. It does guarantee that any subsequent reads from the volume will return zeroes. The ESX driver change is part of the patch. Without it, the struct is missing its initializer and fails to build. I'm not sure what our position on the style of struct initializers is; there are both forms in the code. If this style is heresy, we should add a statement to that effect in HACKING. Dave David Allan (9): Fix ESX storage driver struct initializer Add public API for volume zeroing Define the internal driver API for vol zeroing Implement the public API for vol zeroing Define wire protocol format for vol zeroing Implement RPC client for vol zeroing Implement the remote dispatch bits of vol zeroing Simplified version of volume zeroing based on feedback from the list. Virsh support for vol zeroing daemon/remote.c | 32 + daemon/remote_dispatch_args.h |1 + daemon/remote_dispatch_prototypes.h |8 ++ daemon/remote_dispatch_table.h |5 + include/libvirt/libvirt.h.in|2 + src/driver.h|5 + src/esx/esx_storage_driver.c| 39 +-- src/libvirt.c | 47 src/libvirt_public.syms |1 + src/remote/remote_driver.c | 27 + src/remote/remote_protocol.c| 11 ++ src/remote/remote_protocol.h|9 ++ src/remote/remote_protocol.x|8 +- src/storage/storage_driver.c| 218 +++ tools/virsh.c | 42 +++ 15 files changed, 418 insertions(+), 37 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Format FS pools
* Create the filesystem on the partition used by the pool. --- configure.ac |5 + src/storage/storage_backend_fs.c | 22 ++ 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 743a357..616bd03 100644 --- a/configure.ac +++ b/configure.ac @@ -1252,12 +1252,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([MKE2FS], [mke2fs], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi +if test -z "$MKE2FS" ; then AC_MSG_ERROR([We need mke2fs for FS storage driver]) ; fi else if test -z "$MOUNT" ; then with_storage_fs=no ; fi if test -z "$UMOUNT" ; then with_storage_fs=no ; fi +if test -z "$MKE2FS" ; then with_storage_fs=no ; fi if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi fi @@ -1268,6 +1271,8 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then [Location or name of the mount program]) AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], [Location or name of the mount program]) +AC_DEFINE_UNQUOTED([MKE2FS],["$MKE2FS"], +[Location or name of the mke2fs program]) fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 8279d78..eb5da5e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -45,6 +45,7 @@ #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -500,6 +501,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags ATTRIBUTE_UNUSED) { +const char *mke2fsargv[5], *device = NULL, *format = NULL; int err, ret = -1; char *parent; char *p; @@ -540,6 +542,26 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->target.path); goto error; } + +device = pool->def->source.devices[0].path; +format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format); + +VIR_DEBUG("source device: '%s' format: '%s'", device, format); + +mke2fsargv[0] = MKE2FS; +mke2fsargv[1] = "-t"; +mke2fsargv[2] = format; +mke2fsargv[3] = device; +mke2fsargv[4] = NULL; + +if (virRun(mke2fsargv, NULL) < 0) { +virReportSystemError(errno, + _("Failed to make filesystem of " + "type '%s' on device '%s'"), + format, device); +goto error; +} + ret = 0; error: VIR_FREE(parent); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH [0/1] Filesystem pool formatting
Here's an updated patch based on the feedback from DV and danpb. I haven't added support for any additional filesystems beyond what mke2fs supports. The addition of additional filesystems is easily done and better as a separate patch. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH [0/3] Filesystem pool formatting
I finished these patches this afternoon, and I need to do a bit more testing before I'm comfortable having these patches committed, but I'm sending them along so I can get everybody's feedback overnight. In particular, I'm wondering if my move of the pool unref call is correct. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Add virsh option for format flags
--- tools/virsh.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bd6b6be..de8c67d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4015,6 +4015,7 @@ static const vshCmdInfo info_pool_build[] = { static const vshCmdOptDef opts_pool_build[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("pool name or uuid")}, +{"format", VSH_OT_BOOL, 0, gettext_noop("format the pool (destructive)")}, {NULL, 0, 0, NULL} }; @@ -4023,6 +4024,7 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; int ret = TRUE; +int flags = 0; char *name; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) @@ -4031,7 +4033,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name))) return FALSE; -if (virStoragePoolBuild(pool, 0) == 0) { +if (vshCommandOptBool (cmd, "format")) +flags |= VIR_STORAGE_POOL_CREATE_FORMAT; + +if (virStoragePoolBuild(pool, flags) == 0) { vshPrint(ctl, _("Pool %s built\n"), name); } else { vshError(ctl, _("Failed to build pool %s"), name); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Fixed reference count in pool-build
--- tools/virsh.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e1d1300..bd6b6be 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4036,9 +4036,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, _("Failed to build pool %s"), name); ret = FALSE; -virStoragePoolFree(pool); } +virStoragePoolFree(pool); + return ret; } -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Format FS pools
* If the user supplies the appropriate flag, create the filesystem on the partition used by the pool. --- configure.ac |5 + include/libvirt/libvirt.h.in |3 ++- src/storage/storage_backend_fs.c | 25 - 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 743a357..616bd03 100644 --- a/configure.ac +++ b/configure.ac @@ -1252,12 +1252,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([MKE2FS], [mke2fs], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi +if test -z "$MKE2FS" ; then AC_MSG_ERROR([We need mke2fs for FS storage driver]) ; fi else if test -z "$MOUNT" ; then with_storage_fs=no ; fi if test -z "$UMOUNT" ; then with_storage_fs=no ; fi +if test -z "$MKE2FS" ; then with_storage_fs=no ; fi if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi fi @@ -1268,6 +1271,8 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then [Location or name of the mount program]) AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], [Location or name of the mount program]) +AC_DEFINE_UNQUOTED([MKE2FS],["$MKE2FS"], +[Location or name of the mke2fs program]) fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..a7751b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1053,7 +1053,8 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_RESIZE = 2, /* Extend existing pool */ + VIR_STORAGE_POOL_CREATE_FORMAT = 3 /* Format filesystem during build */ } virStoragePoolBuildFlags; typedef enum { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bbd5787..eedbe2b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -39,12 +39,14 @@ #include #include "virterror_internal.h" +#include "internal.h" #include "storage_backend_fs.h" #include "storage_conf.h" #include "storage_file.h" #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -498,8 +500,9 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { +const char *mke2fsargv[5], *device = NULL, *format = NULL; int err, ret = -1; char *parent; char *p; @@ -540,6 +543,26 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->target.path); goto error; } + +if (flags & VIR_STORAGE_POOL_CREATE_FORMAT) { +device = pool->def->source.devices[0].path; +format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format); + +VIR_DEBUG("source device: '%s' format: '%s'", device, format); + +mke2fsargv[0] = MKE2FS; +mke2fsargv[1] = "-t"; +mke2fsargv[2] = format; +mke2fsargv[3] = device; +mke2fsargv[4] = NULL; + +if (virRun(mke2fsargv, NULL) < 0) { +VIR_ERROR("Failed to make '%s' filesystem on device '%s'", + format, device); +goto error; +} +} + ret = 0; error: VIR_FREE(parent); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Added VIR_ALLOC_VAR macro
Allocates a buffer containing a struct with variable sized array as the last member, useful for some ioctl and other APIs that return data in this way. --- src/util/memory.h | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/src/util/memory.h b/src/util/memory.h index fc9e6c1..e7effd6 100644 --- a/src/util/memory.h +++ b/src/util/memory.h @@ -76,6 +76,24 @@ void virFree(void *ptrptr); #define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count)) /** + * VIR_ALLOC_VAR: + * @ptr: pointer to hold address of allocated memory + * @type: element type of trailing array + * @count: number of array elements to allocate + * + * Allocate sizeof(*ptr) bytes plus an array of 'count' elements, each + * sizeof('type'). This sort of allocation is useful for receiving + * the data of certain ioctls and other APIs which return a struct in + * which the last element is an array of undefined length. The caller + * of this type of API is expected to know the length of the array + * that will be returned and allocate a suitable buffer to contain the + * returned data. + * + * Returns -1 on failure, 0 on success + */ +#define VIR_ALLOC_VAR(ptr, type, count) virAlloc(&(ptr), sizeof(*(ptr)) + (sizeof(type) * count)) + +/** * VIR_REALLOC_N: * @ptr: pointer to hold address of allocated memory * @count: number of elements to allocate -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Build system changes to support fiemap
--- configure.ac|6 ++ src/Makefile.am |7 +++ 2 files changed, 13 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 29c6396..68786c2 100644 --- a/configure.ac +++ b/configure.ac @@ -1438,6 +1438,12 @@ AM_CONDITIONAL([WITH_STORAGE_DISK], [test "$with_storage_disk" = "yes"]) AC_SUBST([LIBPARTED_CFLAGS]) AC_SUBST([LIBPARTED_LIBS]) +AC_CHECK_HEADER([linux/fiemap.h],[ + have_fiemap=yes + AC_DEFINE([HAVE_FIEMAP],[],[fiemap is available]) + ],[]) +AM_CONDITIONAL([HAVE_FIEMAP], [test "$have_fiemap" = "yes"]) + dnl dnl check for libcurl (ESX) dnl diff --git a/src/Makefile.am b/src/Makefile.am index 3232256..92b1b7c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -263,6 +263,9 @@ STORAGE_DRIVER_MPATH_SOURCES = \ STORAGE_DRIVER_DISK_SOURCES = \ storage/storage_backend_disk.h storage/storage_backend_disk.c +STORAGE_DRIVER_FIEMAP_SOURCES =\ + storage/storage_driver_fiemap.c + STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c @@ -661,6 +664,10 @@ if WITH_STORAGE_DISK libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) endif +if HAVE_FIEMAP +libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_FIEMAP_SOURCES) +endif + if WITH_NODE_DEVICES # Needed to keep automake quiet about conditionals if WITH_DRIVER_MODULES -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH [0/3] Volume zeroing
The following patches implement overwriting a volume with zeros when the volume is deleted. The zeroing happens before the delete, so it works for storage backends that don't support actually deleting volumes as well as the ones that do. The intent is that any future VM assigned that volume will not be able to recover any data belonging to the previous VM. It is not intended to prevent attackers with physical access to the medium from recovering data--it simply writes a single pass of zeros over the medium. If the filesystem containing the volume supports the fiemap ioctl and the volume is a sparse file, the volume zeroing code attempts to use fiemap to locate the mapped extents. It does not attempt to zero a sparse file if it cannot use fiemap. Such an operation could take an essentially unbounded amount of time. Since the volume is being deleted, zeroing has less value in the context of backends that support delete, but does provide value with storage backends that do not zero volumes if they are deleted and recreated. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Add volume zeroing
* If the appropriate flag is specified to vol delete, zero out the volume before deleting it. * If the volume is a sparse file and the fiemap ioctl is available, use fiemap to locate the volume's extents. --- src/storage/storage_driver.c| 125 + src/storage/storage_driver.h| 20 + src/storage/storage_driver_fiemap.c | 132 +++ 3 files changed, 277 insertions(+), 0 deletions(-) create mode 100644 src/storage/storage_driver_fiemap.c diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6b1045a..661c412 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include + #if HAVE_PWD_H #include #endif @@ -1518,6 +1521,118 @@ cleanup: return ret; } + +int +storageZeroExtent(virStorageVolDefPtr vol, + struct stat *st, + int fd, + size_t extent_start, + size_t extent_length, + char *writebuf, + size_t *bytes_zeroed) +{ +int ret = -1, written; +size_t remaining, write_size; +char errbuf[64]; + +VIR_DEBUG("extent logical start: %zu len: %zu ", + extent_start, extent_length); + +if (lseek(fd, extent_start, SEEK_SET) < 0) { +VIR_ERROR("Failed to seek to position %zu in volume " + "with path '%s': '%s'", + extent_start, vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +remaining = extent_length; +while (remaining > 0) { + +write_size = (st->st_blksize < remaining) ? st->st_blksize : remaining; +written = safewrite(fd, writebuf, write_size); +if (written < 0) { +VIR_ERROR("Failed to write to storage volume with path '%s': '%s' " + "(attempted to write %d bytes)", + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf)), + write_size); +goto out; +} + +*bytes_zeroed += written; +remaining -= written; +} + +VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", + *bytes_zeroed, vol->target.path); + +ret = 0; + +out: +return ret; +} + + +static int +storageVolumeZeroOut(virStorageVolDefPtr vol) +{ +int ret = -1, fd = -1; +char errbuf[64]; +struct stat st; +char *writebuf; +size_t bytes_zeroed = 0; + +VIR_DEBUG("Zeroing out volume with path '%s'", vol->target.path); + +fd = open(vol->target.path, O_RDWR); +if (fd == -1) { +VIR_ERROR("Failed to open storage volume with path '%s': '%s'", + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +memset(&st, 0, sizeof(st)); + +if (fstat(fd, &st) == -1) { +VIR_ERROR("Failed to stat storage volume with path '%s': '%s'", + vol->target.path, + virStrerror(errno, errbuf, sizeof(errbuf))); +goto out; +} + +if (VIR_ALLOC_N(writebuf, st.st_blksize) != 0) { +virReportOOMError(); +goto out; +} + +/* Don't attempt to brute force a sparse regular file; doing so + * could take an essentially unbounded amount of time. The user + * can always delete and recreate the file to zero it. */ +if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { +ret = storageZeroByFiemap(vol, &st, fd, writebuf); +} else { +ret = storageZeroExtent(vol, +&st, +fd, +0, +vol->allocation, +writebuf, +&bytes_zeroed); +} + +out: +VIR_FREE(writebuf); + +if (fd != -1) { +close(fd); +} + +return ret; +} + + static int storageVolumeDelete(virStorageVolPtr obj, unsigned int flags) { @@ -1563,6 +1678,16 @@ storageVolumeDelete(virStorageVolPtr obj, goto cleanup; } +/* Even if the backend doesn't support volume deletion, we can + * still zero it out; indeed, if the backend does support volume + * deletion, it's almost certain to be faster to delete & recreate + * a volume than it is to zero it out. */ +if (flags & VIR_STORAGE_VOL_DELETE_ZEROED) { +if (storageVolumeZeroOut(vol) == -1) { +goto cleanup; +} +} + if (!backend->deleteVol) { virStorageReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool does not support vol deletion")); diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index 500ea02..cb837f8 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_dr
[libvirt] PATCH [0/1] Fix locking in udev node device backend
The initial udev node device backend implementation fails to take the lock on the node device driverState struct when adding and removing devices. The following patch adds the appropriate locking. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Fix locking for udev device add/remove
* The original udev node device backend neglected to lock the driverState struct containing the device list when adding and removing devices. --- src/node_device/node_device_udev.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2bc2d32..be765c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1190,7 +1190,9 @@ static int udevRemoveOneDevice(struct udev_device *device) int ret = 0; name = udev_device_get_syspath(device); +nodeDeviceLock(driverState); dev = virNodeDeviceFindBySysfsPath(&driverState->devs, name); + if (dev != NULL) { VIR_DEBUG("Removing device '%s' with sysfs path '%s'", dev->def->name, name); @@ -1200,6 +1202,7 @@ static int udevRemoveOneDevice(struct udev_device *device) name); ret = -1; } +nodeDeviceUnlock(driverState); return ret; } @@ -1288,7 +1291,10 @@ static int udevAddOneDevice(struct udev_device *device) goto out; } +nodeDeviceLock(driverState); dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def); +nodeDeviceUnlock(driverState); + if (dev == NULL) { VIR_ERROR("Failed to create device for '%s'", def->name); virNodeDeviceDefFree(def); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Cleanups for udev init code
This patch contains a fix for a segfault when priv is NULL pointed out by Matthias Bolte. --- src/node_device/node_device_udev.c | 42 +++ 1 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index dcd889d..c76c568 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1363,15 +1363,18 @@ static int udevDeviceMonitorShutdown(void) struct udev_monitor *udev_monitor = NULL; struct udev *udev = NULL; -if (driverState) { +if (driverState != NULL) { nodeDeviceLock(driverState); priv = driverState->privateData; -if (priv->watch != -1) -virEventRemoveHandle(priv->watch); +if (priv != NULL) { +if (priv->watch != -1) { +virEventRemoveHandle(priv->watch); +} -udev_monitor = DRV_STATE_UDEV_MONITOR(driverState); +udev_monitor = DRV_STATE_UDEV_MONITOR(driverState); +} if (udev_monitor != NULL) { udev = udev_monitor_get_udev(udev_monitor); @@ -1387,6 +1390,7 @@ static int udevDeviceMonitorShutdown(void) virMutexDestroy(&driverState->lock); VIR_FREE(driverState); VIR_FREE(priv); + } else { ret = -1; } @@ -1547,28 +1551,22 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) { udevPrivate *priv = NULL; struct udev *udev = NULL; -int ret = 0; +int ret = -1; -if (VIR_ALLOC(priv) < 0) { +if (VIR_ALLOC(driverState) < 0) { virReportOOMError(NULL); -ret = -1; goto out; } -priv->watch = -1; - -if (VIR_ALLOC(driverState) < 0) { +if (VIR_ALLOC(priv) < 0) { virReportOOMError(NULL); -VIR_FREE(priv); -ret = -1; goto out; } +priv->watch = -1; + if (virMutexInit(&driverState->lock) < 0) { VIR_ERROR0("Failed to initialize mutex for driverState"); -VIR_FREE(priv); -VIR_FREE(driverState); -ret = -1; goto out; } @@ -1585,10 +1583,8 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) { -VIR_FREE(priv); nodeDeviceUnlock(driverState); VIR_ERROR0("udev_monitor_new_from_netlink returned NULL"); -ret = -1; goto out; } @@ -1608,27 +1604,25 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) priv->watch = virEventAddHandle(udev_monitor_get_fd(priv->udev_monitor), VIR_EVENT_HANDLE_READABLE, udevEventHandleCallback, NULL, NULL); + +nodeDeviceUnlock(driverState); + if (priv->watch == -1) { -nodeDeviceUnlock(driverState); -ret = -1; goto out; } -nodeDeviceUnlock(driverState); - /* Create a fictional 'computer' device to root the device tree. */ if (udevSetupSystemDev() != 0) { -ret = -1; goto out; } /* Populate with known devices */ - if (udevEnumerateDevices(udev) != 0) { -ret = -1; goto out; } +ret = 0; + out: if (ret == -1) { udevDeviceMonitorShutdown(); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Clean up udev init
The following patch contains cleanup for the udev init error handling and Matthias' fix for the case in which priv is NULL when shutdown is called. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Clean up udev init
Matthias' patch made me realize that I didn't write the udev init code as cleanly as I'd like, and the error handling was starting to get a little messy. The attached patch cleans it up. There should be no functional change. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Cleanups for udev init code
--- src/node_device/node_device_udev.c | 34 +- 1 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index dcd889d..6895ac5 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1368,8 +1368,9 @@ static int udevDeviceMonitorShutdown(void) priv = driverState->privateData; -if (priv->watch != -1) +if (priv->watch != -1) { virEventRemoveHandle(priv->watch); +} udev_monitor = DRV_STATE_UDEV_MONITOR(driverState); @@ -1387,6 +1388,7 @@ static int udevDeviceMonitorShutdown(void) virMutexDestroy(&driverState->lock); VIR_FREE(driverState); VIR_FREE(priv); + } else { ret = -1; } @@ -1547,28 +1549,22 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) { udevPrivate *priv = NULL; struct udev *udev = NULL; -int ret = 0; +int ret = -1; -if (VIR_ALLOC(priv) < 0) { +if (VIR_ALLOC(driverState) < 0) { virReportOOMError(NULL); -ret = -1; goto out; } -priv->watch = -1; - -if (VIR_ALLOC(driverState) < 0) { +if (VIR_ALLOC(priv) < 0) { virReportOOMError(NULL); -VIR_FREE(priv); -ret = -1; goto out; } +priv->watch = -1; + if (virMutexInit(&driverState->lock) < 0) { VIR_ERROR0("Failed to initialize mutex for driverState"); -VIR_FREE(priv); -VIR_FREE(driverState); -ret = -1; goto out; } @@ -1585,10 +1581,8 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) { -VIR_FREE(priv); nodeDeviceUnlock(driverState); VIR_ERROR0("udev_monitor_new_from_netlink returned NULL"); -ret = -1; goto out; } @@ -1608,27 +1602,25 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) priv->watch = virEventAddHandle(udev_monitor_get_fd(priv->udev_monitor), VIR_EVENT_HANDLE_READABLE, udevEventHandleCallback, NULL, NULL); + +nodeDeviceUnlock(driverState); + if (priv->watch == -1) { -nodeDeviceUnlock(driverState); -ret = -1; goto out; } -nodeDeviceUnlock(driverState); - /* Create a fictional 'computer' device to root the device tree. */ if (udevSetupSystemDev() != 0) { -ret = -1; goto out; } /* Populate with known devices */ - if (udevEnumerateDevices(udev) != 0) { -ret = -1; goto out; } +ret = 0; + out: if (ret == -1) { udevDeviceMonitorShutdown(); -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Fix daemon-conf invalid failures
The daemon-conf test would fail on my system if there was a system libvirtd running. In the course of troubleshooting that problem, I discovered that the daemon-conf script would always fail if run by itself because it found the line: \# that each "PARAMETER = VALUE" line in this file have the parameter which it mistook for a line containing a parameter. I have changed the test to avoid mistaking a line containing \"PARAMETER = VALUE\" for a parameter line. The corrupted config tests turned out to be failing because the test daemon was discovering the pid file from the running daemon and exiting before it processed the test config file. Specifying the pid file for the corrupt config tests in the same way as for the valid config test solved that problem. --- tests/daemon-conf |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/daemon-conf b/tests/daemon-conf index 1eb4be1..10c1628 100755 --- a/tests/daemon-conf +++ b/tests/daemon-conf @@ -19,7 +19,7 @@ grep '^#define WITH_QEMU 1' "$CONFIG_HEADER" > /dev/null || conf="$abs_top_srcdir/daemon/libvirtd.conf" # Ensure that each commented out PARAMETER = VALUE line has the expected form. -grep '[a-z_] *= *[^ ]' "$conf" | grep -vE '^#[a-z_]+ = ' \ +grep -v '\"PARAMETER = VALUE\"' "$conf" | grep '[a-z_] *= *[^ ]' | grep -vE '^#[a-z_]+ = ' \ && { echo "$0: found unexpected lines (above) in $conf" 1>&2; exit 1; } # Start with the sample libvirtd.conf file, uncommenting all real directives. @@ -45,7 +45,7 @@ while :; do esac # Run libvirtd, expecting it to fail. - $abs_top_builddir/daemon/libvirtd --config=$f 2> err && fail=1 + $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=$f 2> err && fail=1 case $rhs in # '"'*) msg='should be a string';; -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] A couple of problems with the daemon-conf test
Here's a patch fixing two problems I found with the daemon-conf test that prevented the test from passing if the system libvirt was running on my system. The first change only affects the direct running of the daemon-conf script, i.e, if someone executes ./daemon-conf, which will always exit failure because the default config file contains a line that the test believes should not be there. The invocation by make-check supplies a different config file that's not subject to that problem. The second problem is that the corrupt config tests don't supply the pid-file argument to libvirt, so on my system it was attempting to use the same pidfile as the running system daemon and failing all the tests except the valid config test. Supplying the --pid-file argument when running the corrupt config tests as it is supplied for the valid config tests fixes the problem. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Addtions to .gitignore
--- build-aux/.gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/build-aux/.gitignore b/build-aux/.gitignore index af7a347..043aafa 100644 --- a/build-aux/.gitignore +++ b/build-aux/.gitignore @@ -5,3 +5,4 @@ /gitlog-to-changelog /useless-if-before-free /vc-list-files +/warn-on-use.h -- 1.6.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] MultiIQN support
This patch implements multiIQN support for iSCSI pools, allowing the user to specify the initiator IQN that should be used when creating the pool. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Implement support for multi IQN
In other words, permit the initiator to use a variety of IQNs rather than just the system IQN when creating iSCSI pools. --- docs/schemas/storagepool.rng| 12 ++ src/conf/storage_conf.c | 16 ++- src/conf/storage_conf.h |9 ++ src/storage/storage_backend_iscsi.c | 253 +- src/storage/storage_backend_iscsi.h |4 + 5 files changed, 280 insertions(+), 14 deletions(-) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 249bf9c..247664e 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -188,6 +188,15 @@ + + + + + + + + + @@ -363,6 +372,9 @@ + + + diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ea49531..f56abdd 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -106,11 +106,12 @@ struct _virStorageVolOptions { /* Flags to indicate mandatory components in the pool source */ enum { -VIR_STORAGE_POOL_SOURCE_HOST= (1<<0), -VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), -VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), -VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), -VIR_STORAGE_POOL_SOURCE_NAME= (1<<4), +VIR_STORAGE_POOL_SOURCE_HOST= (1<<0), +VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), +VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), +VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), +VIR_STORAGE_POOL_SOURCE_NAME= (1<<4), +VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN = (1<<5), }; @@ -179,7 +180,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { { .poolType = VIR_STORAGE_POOL_ISCSI, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_HOST | - VIR_STORAGE_POOL_SOURCE_DEVICE), + VIR_STORAGE_POOL_SOURCE_DEVICE | + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN), }, .volOptions = { .formatToString = virStoragePoolFormatDiskTypeToString, @@ -283,6 +285,7 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) { VIR_FREE(source->dir); VIR_FREE(source->name); VIR_FREE(source->adapter); +VIR_FREE(source->initiator.iqn); if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) { VIR_FREE(source->auth.chap.login); @@ -421,6 +424,7 @@ virStoragePoolDefParseSource(virConnectPtr conn, } source->host.name = virXPathString(conn, "string(./host/@name)", ctxt); +source->initiator.iqn = virXPathString(conn, "string(./initiator/iqn)", ctxt); nsource = virXPathNodeSet(conn, "./device", ctxt, &nodeset); if (nsource > 0) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index a795981..a5f0100 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -182,6 +182,12 @@ struct _virStoragePoolSourceDeviceExtent { int type; /* free space type */ }; +typedef struct _virStoragePoolSourceInitiatorAttr virStoragePoolSourceInitiatorAttr; +struct _virStoragePoolSourceInitiatorAttr { +/* Initiator IQN */ +char *iqn; +}; + /* * Pools can be backed by one or more devices, and some * allow us to track free space on underlying devices. @@ -223,6 +229,9 @@ struct _virStoragePoolSource { /* Or a name */ char *name; +/* Initiator IQN */ +virStoragePoolSourceInitiatorAttr initiator; + int authType; /* virStoragePoolAuthType */ union { virStoragePoolAuthChap chap; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b516add..e0788cf 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -33,16 +33,17 @@ #include #include #include +#include #include "virterror_internal.h" #include "storage_backend_scsi.h" #include "storage_backend_iscsi.h" #include "util.h" #include "memory.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE - static int virStorageBackendISCSITargetIP(virConnectPtr conn, const char *hostname, @@ -153,21 +154,257 @@ virStorageBackendISCSISession(virConnectPtr conn, return session; } + +#define LINE_SIZE 4096 + +static int +virStorageBackendIQNFound(virConnectPtr conn, + virStoragePoolObjPtr pool, + char **ifacename) +{ +int ret = IQN_MISSING, fd = -1; +char ebuf[64]; +FILE *fp = NULL; +pid_t child = 0; +char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL, +*saveptr = NULL; +const char *const prog[] = { +ISCSIADM, "--mode", "iface", NULL +}; + +if (VIR_ALLOC_N(line, LINE_SIZE) != 0) { +ret = IQN_ERROR; +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not allocate memory for output of '%s'"), +
[libvirt] [PATCH 0/1] Fix log level message
This oneliner corrects a debug message that was incorrectly being logged as an error. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Corrected log level of WWN path message
* It was incorrectly set to ERROR; fixed by setting to DEBUG --- src/node_device/node_device_linux_sysfs.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index b1dff10..c792380 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -51,7 +51,7 @@ static int open_wwn_file(const char *prefix, /* fd will be closed by caller */ if ((*fd = open(wwn_path, O_RDONLY)) != -1) { -VIR_ERROR(_("Opened WWN path '%s' for reading"), +VIR_DEBUG(_("Opened WWN path '%s' for reading"), wwn_path); } else { VIR_ERROR(_("Failed to open WWN path '%s' for reading"), -- 1.6.5.5 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Clarify error message
* If a user specified a number of devices not equal to one, the error message could be misleading. --- src/storage/storage_backend_fs.c | 12 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ca1f4cb..7a36c57 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -366,7 +366,10 @@ virStorageBackendFileSystemMount(virConnectPtr conn, } else { if (pool->def->source.ndevice != 1) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("missing source device")); + _("Exactly one source device " +"may be specified for pool type '%s'"), + virStoragePoolFormatFileSystemTypeToString( + pool->def->source.format)); return -1; } } @@ -518,9 +521,10 @@ virStorageBackendFileSystemBuild(virConnectPtr conn, if (pool->def->source.ndevice != 1) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "Exactly one source device " - "may be specified for pool type '%s'", - virStoragePoolFormatFileSystemTypeToString(pool->def->source.format)); + _("Exactly one source device " +"may be specified for pool type '%s'"), + virStoragePoolFormatFileSystemTypeToString( + pool->def->source.format)); goto out; } -- 1.6.5.5 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Make filesystems during fs pool build
This patch implements the formatting of block devices in the pool build operation of filesystem pools. The existing implementation would only make the directory on which to mount the filesystem, but required the user to make the filesystem manually. My only concern with this approach is that it changes the build behavior in a way that's potentially destructive, so I wonder whether it would be worth adding a flag to enable this behavior. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Format fs pool during pool build
--- configure.in |5 + src/storage/storage_backend_fs.c | 36 +--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/configure.in b/configure.in index 3f2f8ff..59ceadd 100644 --- a/configure.in +++ b/configure.in @@ -1243,12 +1243,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([MKE2FS], [mke2fs], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi +if test -z "$MKE2FS" ; then AC_MSG_ERROR([We need mke2fs for FS storage driver]) ; fi else if test -z "$MOUNT" ; then with_storage_fs=no ; fi if test -z "$UMOUNT" ; then with_storage_fs=no ; fi +if test -z "$MKE2FS" ; then with_storage_fs=no ; fi if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi fi @@ -1259,6 +1262,8 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then [Location or name of the mount program]) AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], [Location or name of the mount program]) +AC_DEFINE_UNQUOTED([MKE2FS],["$MKE2FS"], +[Location or name of the mke2fs program]) fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b7d4bd6..ca1f4cb 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -45,6 +45,7 @@ #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -505,15 +506,44 @@ virStorageBackendFileSystemBuild(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags ATTRIBUTE_UNUSED) { -int err; +const char *mke2fsargv[5], *device = NULL, *format = NULL; +int err, ret = -1; + if ((err = virFileMakePath(pool->def->target.path)) < 0) { virReportSystemError(conn, err, _("cannot create path '%s'"), pool->def->target.path); -return -1; +goto out; } -return 0; +if (pool->def->source.ndevice != 1) { +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "Exactly one source device " + "may be specified for pool type '%s'", + virStoragePoolFormatFileSystemTypeToString(pool->def->source.format)); +goto out; +} + +device = pool->def->source.devices[0].path; +format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format); + +VIR_DEBUG("source device: '%s' format: '%s'", device, format); + +mke2fsargv[0] = MKE2FS; +mke2fsargv[1] = "-t"; +mke2fsargv[2] = format; +mke2fsargv[3] = device; +mke2fsargv[4] = NULL; + +if (virRun(conn, mke2fsargv, NULL) < 0) { +VIR_ERROR("Failed to make '%s' filesystem on device '%s'", format, device); +goto out; +} + +ret = 0; + +out: +return ret; } -- 1.6.5.5 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Don't update vol details after build
* This patch removes the call to vol update after the volume build completes. The update call is currently meaningless anyway because the vol build is passed a copy of the definition, so the update result is thrown away. More importantly, if the user specified a selinux label for the volume, the update call results in a double free of the label. --- src/storage/storage_backend_fs.c |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b7d4bd6..4fe40b3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -822,14 +822,6 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; } -/* Refresh allocation / permissions info, but not capacity */ -if (virStorageBackendUpdateVolTargetInfoFD(conn, &vol->target, fd, - &vol->allocation, - NULL) < 0) { -close(fd); -return -1; -} - if (close(fd) < 0) { virReportSystemError(conn, errno, _("cannot close file '%s'"), -- 1.6.5.5 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Don't update vol details after build
This patch fixes the bug reported at: https://bugzilla.redhat.com/show_bug.cgi?id=510450 See the patch for details. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] Add a simple pool type for multipath devices
This pool type contains volumes for the multipath devices that are present on the host. It does not (yet) support any sort of multipath configuration, so that aspect of system administration must still be done at host build time. --- configure.in| 24 +++ po/POTFILES.in |1 + src/Makefile.am |9 + src/storage_backend.c | 82 ++ src/storage_backend.h |5 +- src/storage_backend_mpath.c | 344 +++ src/storage_backend_mpath.h | 31 src/storage_conf.c |7 +- src/storage_conf.h |1 + 9 files changed, 502 insertions(+), 2 deletions(-) create mode 100644 src/storage_backend_mpath.c create mode 100644 src/storage_backend_mpath.h diff --git a/configure.in b/configure.in index 634e812..7a94373 100644 --- a/configure.in +++ b/configure.in @@ -906,6 +906,8 @@ AC_ARG_WITH([storage-iscsi], [ --with-storage-iscsiwith iSCSI backend for the storage driver (on)],[],[with_storage_iscsi=check]) AC_ARG_WITH([storage-scsi], [ --with-storage-scsi with SCSI backend for the storage driver (on)],[],[with_storage_scsi=check]) +AC_ARG_WITH([storage-mpath], +[ --with-storage-mpathwith mpath backend for the storage driver (on)],[],[with_storage_mpath=check]) AC_ARG_WITH([storage-disk], [ --with-storage-disk with GPartd Disk backend for the storage driver (on)],[],[with_storage_disk=check]) @@ -916,6 +918,7 @@ if test "$with_libvirtd" = "no"; then with_storage_lvm=no with_storage_iscsi=no with_storage_scsi=no + with_storage_mpath=no with_storage_disk=no fi if test "$with_storage_dir" = "yes" ; then @@ -1037,6 +1040,26 @@ if test "$with_storage_scsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +if test "$with_storage_mpath" = "check"; then + with_storage_mpath=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, + [whether mpath backend for storage driver is enabled]) +fi +AM_CONDITIONAL([WITH_STORAGE_MPATH], [test "$with_storage_mpath" = "yes"]) + +if test "$with_storage_mpath" = "yes"; then + DEVMAPPER_REQUIRED=0.0 + DEVMAPPER_CFLAGS= + DEVMAPPER_LIBS= + PKG_CHECK_MODULES(DEVMAPPER, devmapper >= $DEVMAPPER_REQUIRED, +[], [ +AC_MSG_ERROR( +[You must install device-mapper-devel >= $DEVMAPPER_REQUIRED to compile libvirt]) +]) +fi +AC_SUBST([DEVMAPPER_CFLAGS]) +AC_SUBST([DEVMAPPER_LIBS]) LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1506,6 +1529,7 @@ AC_MSG_NOTICE([ NetFS: $with_storage_fs]) AC_MSG_NOTICE([ LVM: $with_storage_lvm]) AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi]) AC_MSG_NOTICE([SCSI: $with_storage_scsi]) +AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([Disk: $with_storage_disk]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 7ccf3ab..f70936b 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -36,6 +36,7 @@ src/storage_backend_fs.c src/storage_backend_iscsi.c src/storage_backend_logical.c src/storage_backend_scsi.c +src/storage_backend_mpath.c src/storage_conf.c src/storage_driver.c src/test.c diff --git a/src/Makefile.am b/src/Makefile.am index 79826b1..57bf9ba 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -184,6 +184,9 @@ STORAGE_DRIVER_ISCSI_SOURCES = \ STORAGE_DRIVER_SCSI_SOURCES = \ storage_backend_scsi.h storage_backend_scsi.c +STORAGE_DRIVER_MPATH_SOURCES = \ + storage_backend_mpath.h storage_backend_mpath.c + STORAGE_DRIVER_DISK_SOURCES = \ storage_backend_disk.h storage_backend_disk.c @@ -436,6 +439,10 @@ if WITH_STORAGE_SCSI libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SCSI_SOURCES) endif +if WITH_STORAGE_MPATH +libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_MPATH_SOURCES) +endif + if WITH_STORAGE_DISK libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) endif @@ -495,6 +502,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_LVM_SOURCES) \ $(STORAGE_DRIVER_ISCSI_SOURCES) \ $(STORAGE_DRIVER_SCSI_SOURCES) \ + $(STORAGE_DRIVER_MPATH_SOURCES) \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ @@ -563,6 +571,7 @@ libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \ $(COVERAGE_CFLAGS:-f%=-Wc,-f%) \ $(LIBXML_LIBS) $(SELINUX_LIBS) \ $(XEN_LIBS) $(DRIVER_MODULE_LIBS) \ + $(DEVMAPPER_LIBS) \
[libvirt] [PATCH 0/1] Multipath pool support
The following patch implements multipath pool support. It's very basic functionality, consisting of creating a pool that contains all the multipath devices on the host. That will cover the common case of users who just want to discover all the available multipath devices and assign them to guests. It doesn't currently allow configuration of multipathing, so for now setting the multipath configuration will have to continue to be done as part of the host system build. Example XML to create the pool is: mpath /dev/mapper The target element is ignored, as it is by the disk pool, but the config code rejects the XML if it does not exist. That behavior should obviously be cleaned up, but I think that should be done in a separate patch, as it's really a bug in the config code, not related to the addition of the new pool type. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Fix pool create when pool already exists
When storagePoolCreate is called to define a pool that already exists, it mistakenly destroys the existing pool. This patch fixes the problem. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list