[libvirt] [PATCH v2 3/5] Add a helper API for setting up a NBD device with qemu-nbd

[Eric Blake]

On 04/22/2013 08:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Add a virFileNBDDeviceAssociate method, which given a filename
> will setup a NBD device, using qemu-nbd as the server.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/libvirt_private.syms |   1 +
>  src/util/virfile.c       | 151 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virfile.h       |   6 ++
>  3 files changed, 155 insertions(+), 3 deletions(-)
> 

> +++ b/src/util/virfile.c
> @@ -530,6 +530,7 @@ static int virFileLoopDeviceOpen(char **dev_name)
>          goto cleanup;
>      }
>  
> +    errno = 0;
>      while ((de = readdir(dh)) != NULL) {
>          if (!STRPREFIX(de->d_name, "loop"))
>              continue;
> @@ -561,10 +562,15 @@ static int virFileLoopDeviceOpen(char **dev_name)
>          /* Oh well, try the next device */
>          VIR_FORCE_CLOSE(fd);
>          VIR_FREE(looppath);
> +        errno = 0;
>      }
>  
> -    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                   _("Unable to find a free loop device in /dev"));
> +    if (errno != 0)
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to iterate over loop devices"));
> +    else
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Unable to find a free loop device in /dev"));

Hmm, this looks like an independent (but useful) cleanup.


> +static char *
> +virFileNBDDeviceFindUnused(void)
> +{
> +    DIR *dh;
> +    char *ret = NULL;
> +    struct dirent *de;
> +
> +    if (!(dh = opendir(SYSFS_BLOCK_DIR))) {
> +        virReportSystemError(errno,
> +                             _("Cannot read directory %s"),
> +                             SYSFS_BLOCK_DIR);
> +        return NULL;
> +    }
> +
> +    while ((de = readdir(dh)) != NULL) {

Oops, you're missing errno checking here (readdir can be such a pain to
use correctly).

> +    cmd = virCommandNew(qemunbd);
> +
> +    /* Explicitly not trying to cope with old qemu-nbd which
> +     * lacked --format. We want to see a fatal error in that
> +     * case since it would be security flaw to continue */
> +    if (fmtstr) {
> +        virCommandAddArg(cmd, "--format");
> +        virCommandAddArg(cmd, fmtstr);
> +    }

You could shorten if you wanted:

if (fmtstr)
    virCommandAddArgList(cmd, "--format", fmtstr, NULL);


>  #else /* __linux__ */
>  
>  int virFileLoopDeviceAssociate(const char *file,
> -                               char **dev ATTRIBUTE_UNUSED)
> +                               char **dev ATTRIBUTE)

Whoa - that can't be right.

The idea is good, but the patch isn't quite perfect.  Are you still
shooting for 1.0.5?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130430/23e60ccd/attachment-0001.sig>