On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
> When libvirt starts up all storage pools default to the inactive
> state, even if the underlying storage is already active on the
> host. This introduces a new API into the internal storage backend
> drivers that checks whether a storage pool is already active. If
> the pool is active at libvirtd startup, the volume list will be
> immediately populated.
>
> +++ b/src/storage/storage_backend_mpath.c
> @@ -27,6 +27,8 @@
> #include <stdio.h>
> #include <dirent.h>
> #include <fcntl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
<sys/types.h> is generally redundant, now that POSIX 2008 requires most
headers to be self-contained.
> +static int
> +virStorageBackendMpathCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> + bool *isActive)
> +{
> + const char *path = "/dev/mpath";
> +
> + *isActive = false;
> +
> + struct stat sb;
> + if (stat(path, &sb) == 0)
> + *isActive = true;
General observation: using stat() for existence checks is generally
slower than access(,F_OK), because the kernel has to do more work to
populate the result buffer that you then ignore. While what you have
works, maybe you should consider rewriting this to use access().
> +++ b/src/storage/storage_backend_scsi.c
> @@ -27,6 +27,9 @@
> #include <stdio.h>
> #include <dirent.h>
> #include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
Why <sys/wait.h>?
> +++ b/src/storage/storage_driver.c
> @@ -68,17 +68,29 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
>
> for (i = 0 ; i < driver->pools.count ; i++) {
> virStoragePoolObjPtr pool = driver->pools.objs[i];
> + virStorageBackendPtr backend;
> + bool started = false;
>
> virStoragePoolObjLock(pool);
> - if (pool->autostart &&
> - !virStoragePoolObjIsActive(pool)) {
> - virStorageBackendPtr backend;
> - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
> - VIR_ERROR(_("Missing backend %d"), pool->def->type);
> - virStoragePoolObjUnlock(pool);
> - continue;
> - }
> + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
> + VIR_ERROR(_("Missing backend %d"), pool->def->type);
> + virStoragePoolObjUnlock(pool);
> + continue;
> + }
>
> + if (backend->checkPool &&
> + backend->checkPool(NULL, pool, &started) < 0) {
> + virErrorPtr err = virGetLastError();
> + VIR_ERROR(_("Failed to initialize storage pool '%s': %s"),
> + pool->def->name, err ? err->message :
> + "no error message found");
Missing _() around that last string.
ACK to the overall idea, but you may want to respin this given my comments.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list