Re: [libvirt] [PATCH 4/7] Add a helper API for setting up a NBD device with qemu-nbd
On 03/19/2013 07:39 AM, John Ferlan wrote:
On 03/15/2013 12:32 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)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(a)redhat.com>
> ---
> +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) {
readdir() is an annoying interface. To properly distinguish errors, you
have to assign errno to 0 before each iteration, then check if errno is
set when the while loop terminates.
> + if (STRPREFIX(de->d_name, "nbd")) {
> + int rv = virFileNBDDeviceIsBusy(de->d_name);
> + if (rv < 0)
> + goto cleanup;
> + if (rv == 0) {
> + if (virAsprintf(&ret, "/dev/%s", de->d_name) <
0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> + goto cleanup;
> + }
> + }
> + }
> +
> + virReportSystemError(EBUSY, "%s",
> + _("No free NBD devices"));
Your choice of error reporting is to blindly overwrite any (potential)
error from readdir. That potentially loses information. Then again,
since you normally succeed only by finding a winning existing entry, and
since you are at least giving a reasonable error message, even if you
overwrote the readdir() errno, I can live with it.
> +int virFileNBDDeviceAssociate(const char *file,
> + char **dev ATTRIBUTE_UNUSED,
> + bool readonly ATTRIBUTE_UNUSED)
> +{
> + virReportSystemError(ENOSYS,
> + _("Unable to associate file %s with NBD
device"),
> + file);
> + *dev = NULL;
Since this is done - should this still be UNUSED in the header?
It doesn't hurt to leave it in, but it also doesn't hurt to take out the
ATTRIBUTE_UNUSED or even drop the '*dev = NULL' line.
ACK in any case.
Agreed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)