On 04/22/2013 08:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list