On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
> The SCSI volumes currently get a name like '17:0:0:1' based
> on $host:$bus:$target:$lun. The names are intended to be unique
> per pool and stable across pool restarts. The inclusion of the
> $host component breaks this, because the $host number for iSCSI
> pools is dynamically allocated by the kernel at time of login.
> This changes the name to be 'unit:0:0:1', ie removes the leading
> host component. THe 'unit:' prefix is just to ensure the volume
s/THe/The/
> name doesn't start with a number and make it clearer when seen
> out of context.
>
> The SCSI volumes also get a 'key' field based on the fully
> qualified volume path. All SCSI volumes have a unique serial
> available in hardware which can be obtained by sending a
> suitable SCSI command. Call out to udev's 'scsi_id' command
> to fetch this value
I don't know if you adequately answered the usage questions raised by
others, but from the code review aspect, I hadn't seen an ack yet.
>
> * src/storage/storage_backend_scsi.c: Improve key and name
> field value stability and uniqness
s/uniqness/uniqueness/
> +static char *
> +virStorageBackendSCSISerial(const char *dev)
> +{
> + const char *cmdargv[] = {
> + "/lib/udev/scsi_id",
> + "--replace-whitespace",
> + "--whitelisted",
> + "--device", dev,
> + NULL
> + };
> + int fd = -1;
> + pid_t child;
> + FILE *list = NULL;
> + char line[1024];
> + char *serial = NULL;
> + int err;
> + int exitstatus;
> +
> + /* Run the program and capture its output */
> + if (virExec(cmdargv, NULL, NULL,
> + &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0)
> + goto cleanup;
All the more reason for me to get my virCommand patch cleaned up per
comments and pushed. This patch seems better off to rebase on top of
virCommand.
> +
> + if ((list = fdopen(fd, "r")) == NULL) {
VIR_FDOPEN (hmm, I really need to get that promised syntax-check going
for fdopen/[f]close).
> static int
> virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
> - uint32_t host,
> + uint32_t host ATTRIBUTE_UNUSED,
> uint32_t bus,
> uint32_t target,
> uint32_t lun,
> @@ -183,7 +240,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>
> vol->type = VIR_STORAGE_VOL_BLOCK;
>
> - if (virAsprintf(&(vol->name), "%u.%u.%u.%u", host, bus, target, lun) < 0) {
> + /* 'host' is dynamically allocated by the kernel, first come,
> + * first served, per HBA. As such it isn't suitable for use
> + * in the volume name. We only need uniquness per-pool, so
s/uniquness/uniqueness/ (cute - two unique mis-spellings for the same
word :)
--
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