On Wed, Sep 02, 2009 at 11:28:27AM -0400, Dave Allan wrote:
@@ -1177,6 +1180,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])
Need to update livbvirt.spec.in with a dependancy on device mapper for this tool, both Requires & BuildRequries
diff --git a/src/Makefile.am b/src/Makefile.am index bedeb84..cf3420b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -199,6 +199,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
@@ -478,6 +481,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 @@ -539,6 +546,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) \ @@ -607,6 +615,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) \ @CYGWIN_EXTRA_LDFLAGS@ @MINGW_EXTRA_LDFLAGS@ libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) libvirt.syms
I think you probably need a $(DEVMAPPER_CFLAGS) somewhere in here too, even if it happens to be empty for default Fedora installs.
+static int +virStorageBackendMpathUpdateVolTargetInfo(virConnectPtr conn, + virStorageVolTargetPtr target, + unsigned long long *allocation, + unsigned long long *capacity) +{ + int ret = 0; + int fd = -1; + + if ((fd = open(target->path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), + target->path); + ret = -1; + goto out; + } + + if (virStorageBackendUpdateVolTargetInfoFD(conn, + target, + fd, + allocation, + capacity) < 0) { + + VIR_ERROR(_("Failed to update volume target info for %s"), + target->path); + + ret = -1; + goto out; + } + + if (virStorageBackendUpdateVolTargetFormatFD(conn, + target, + fd) < 0) { + VIR_ERROR(_("Failed to update volume target format for %s"), + target->path); + + ret = -1; + goto out; + }
These two VIR_EROR calls should be virRaiseError() or similar if we're going to return a fail status.
+static int +virStorageBackendMpathNewVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + const int devnum, + const char *dev) +{ + virStorageVolDefPtr vol; + int retval = 0; + + if (VIR_ALLOC(vol) < 0) { + virReportOOMError(conn); + retval = -1; + goto out; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + if (virAsprintf(&(vol->name), "dm-%u", devnum) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + if (virStorageBackendMpathUpdateVolTargetInfo(conn, + &vol->target, + &vol->allocation, + &vol->capacity) < 0) { + + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to update volume for '%s'"), + vol->target.path); + retval = -1; + goto free_vol; + } + + /* XXX should use logical unit's UUID instead */ + vol->key = strdup(vol->target.path); + if (vol->key == NULL) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + + pool->def->capacity += vol->capacity; + pool->def->allocation += vol->allocation; + + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count + 1) < 0) { + virReportOOMError(conn); + retval = -1; + goto free_vol; + } + pool->volumes.objs[pool->volumes.count++] = vol; +
If 'retval' is declared as -1 initially, then
+ goto out; + +free_vol: + virStorageVolDefFree(vol); +out: + return retval; +}
could be simplified with ret = 0; cleanup: if (ret != 0) virStorageVolDefFree(vol); return ret; Allowing the removal of all the retval = -1 assignments during the method.
+ } + + if (!(strcmp(target_type, "multipath"))) {
Can use STRNEQ() here ACK if those minor things are cleaned up Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|