Re: [Libguestfs] [PATCH] Fix broken qemu <= 0.10 which randomly adds a CD-ROM device to the appliance
Richard W.M. Jones wrote:
>From 6bb26f2d253b77570c2b9291021db4364597b6df Mon Sep 17 00:00:00
2001
From: Richard Jones <rjones(a)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(a)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