[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: 13/16: iSCSI backend



On Tue, Feb 12, 2008 at 04:39:25AM +0000, Daniel P. Berrange wrote:
> This provides a storage pool using the iSCSI protocol. Since there
> is no API for iSCSI it is implemented by simply shelling out to the
> iscsiadm command line tool. A pool corresponds to a single target
> on the iSCSI server. Starting a pool logs into the server and maps
> the target's LUNs into the local filesystem. The default nodes are
> under /dev, allocated-on-demand and thus not guarenteed to be stable
> across reboots. For this reason it is recommended by the pool target
> path be configured to point to /dev/disk/by-path or /dev/disk/by-id
> whose entries are guarenteed stable for lifetime of the target+LUN.
> The 'refresh' operation will rescan the target for new LUNs and purge
> old LUNs allowing dynamic updates without needing a pool restart.

  Okay, the portability of this code will be fun, but that
makes sense as is,


> diff -r 9a3200af0a3d libvirt.spec.in
> --- a/libvirt.spec.in	Thu Feb 07 11:14:56 2008 -0500
> +++ b/libvirt.spec.in	Thu Feb 07 11:34:46 2008 -0500
> @@ -51,6 +51,8 @@ Requires: /usr/sbin/qcow-create
>  %endif
>  # For LVM drivers
>  Requires: lvm2
> +# For ISCSI driver
> +Requires: iscsi-initiator-utils

  Apparently iscsi-initiator-utils exists in RHEL 4 (and 3 but
we are not tergetting that old) it would be great if that worked
there too.

[...]
> +static int virStorageBackendISCSIMakeLUN(virConnectPtr conn,
> +                                         virStoragePoolObjPtr pool,
> +                                         char **const groups,
> +                                         void *data ATTRIBUTE_UNUSED)
[...]
> +    if ((devpath = malloc(5 + strlen(dev) + 1)) == NULL) {
> +        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "devpath");
> +        goto cleanup;
> +    }
> +    strcpy(devpath, "/dev/");
> +    strcat(devpath, dev);

  each time i see strcat() I get afraid, a good old snprintf for those
2 is probably easier to understand.

  Looks fine to me, i never played with iSCSI, so no real insight on the
questions left in the code.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]