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@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@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