[Libguestfs] [PATCH] Fix broken qemu <= 0.10 which randomly adds a CD-ROM device to the appliance

[Jim Meyering]

Richard W.M. Jones wrote:
>>From 6bb26f2d253b77570c2b9291021db4364597b6df Mon Sep 17 00:00:00 2001
> From: Richard Jones <rjones at centos5x32.home.annexia.org>
> Date: Wed, 29 Jul 2009 15:02:16 +0100
> Subject: [PATCH] Don't show empty CD devices (RHBZ#514505).
>
> ---
>  daemon/devsparts.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/daemon/devsparts.c b/daemon/devsparts.c
> index 32d2fa8..a198ccd 100644
> --- a/daemon/devsparts.c
> +++ b/daemon/devsparts.c
> @@ -37,6 +37,7 @@ do_list_devices (void)
>    DIR *dir;
>    struct dirent *d;
>    char buf[256];
> +  int fd;
>
>    dir = opendir ("/sys/block");
>    if (!dir) {
> @@ -49,6 +50,18 @@ do_list_devices (void)
>  	strncmp (d->d_name, "hd", 2) == 0 ||
>  	strncmp (d->d_name, "vd", 2) == 0) {
>        snprintf (buf, sizeof buf, "/dev/%s", d->d_name);
> +
> +      /* RHBZ#514505: Some versions of qemu <= 0.10 device to add a
> +       * CD-ROM device even though we didn't request it.  Try to
> +       * detect this by seeing if the device contains media.
> +       */
> +      fd = open (buf, O_RDONLY);

This all looks fine.  (modulo s/device to // in the comment)
However, imho it would be slightly better
(easier to read/review) if "fd" were declared at least
in the inner scope where it is used.
Personally, I'd go all the way and do this:

         int fd = open (buf, O_RDONLY);

assuming you're ok with c99 decl-after-stmt.

Hoping I'm not being too presumptuous, here's a patch:

>From aed1ab4721ab4b60081f539bd0023f64616ccd7e Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 29 Jul 2009 14:48:45 -0400
Subject: [PATCH] fix comments; move declarations

* daemon/devsparts.c (do_list_devices, do_list_partitions):
Remove stray words in comments.
Move declarations down to definition.
---
 daemon/devsparts.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index a198ccd..33579ba 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -37,7 +37,6 @@ do_list_devices (void)
   DIR *dir;
   struct dirent *d;
   char buf[256];
-  int fd;

   dir = opendir ("/sys/block");
   if (!dir) {
@@ -51,11 +50,11 @@ do_list_devices (void)
 	strncmp (d->d_name, "vd", 2) == 0) {
       snprintf (buf, sizeof buf, "/dev/%s", d->d_name);

-      /* RHBZ#514505: Some versions of qemu <= 0.10 device to add a
+      /* RHBZ#514505: Some versions of qemu <= 0.10 add a
        * CD-ROM device even though we didn't request it.  Try to
        * detect this by seeing if the device contains media.
        */
-      fd = open (buf, O_RDONLY);
+      int fd = open (buf, O_RDONLY);
       if (fd == -1) {
 	perror (buf);
 	continue;
@@ -92,7 +91,6 @@ do_list_partitions (void)
   DIR *dir, *dir2;
   struct dirent *d;
   char buf[256], devname[256];
-  int fd;

   dir = opendir ("/sys/block");
   if (!dir) {
@@ -106,11 +104,11 @@ do_list_partitions (void)
 	strncmp (d->d_name, "vd", 2) == 0) {
       snprintf (buf, sizeof buf, "/dev/%s", d->d_name);

-      /* RHBZ#514505: Some versions of qemu <= 0.10 device to add a
+      /* RHBZ#514505: Some versions of qemu <= 0.10 add a
        * CD-ROM device even though we didn't request it.  Try to
        * detect this by seeing if the device contains media.
        */
-      fd = open (buf, O_RDONLY);
+      int fd = open (buf, O_RDONLY);
       if (fd == -1) {
 	perror (buf);
 	continue;
--
1.6.3.3