8:19 a.m.
Return a list of Linux MD devices detected in the guest.
This API complements list_devices, list_partitions, list_lvs and
list_dm_devices.
---
daemon/Makefile.am | 1 +
daemon/daemon.h | 1 +
daemon/guestfsd.c | 23 +++++++---
daemon/md.c | 75 +++++++++++++++++++++++++++++++++++
generator/generator_actions.ml | 10 +++++
po/POTFILES.in | 1 +
regressions/Makefile.am | 1 +
regressions/test-list-md-devices.sh | 59 +++++++++++++++++++++++++++
src/MAX_PROC_NR | 2 +-
9 files changed, 165 insertions(+), 8 deletions(-)
create mode 100644 daemon/md.c
create mode 100755 regressions/test-list-md-devices.sh
8:19 a.m.
New subject: [PATCH 2/2] Update list-filesystems to check md devices
---
regressions/test-list-filesystems.sh | 77 ++++++++++++++++++++++++++++++++++
src/listfs.c | 60 ++++++++++++++-------------
2 files changed, 108 insertions(+), 29 deletions(-)
create mode 100755 regressions/test-list-filesystems.sh
9:01 a.m.
New subject: [PATCH 2/2] Update list-filesystems to check md devices
This looks fine, modulo binding the mdadm -C call as I said
in the other email.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
10:06 a.m.
New subject: Missed test script from Makefile.am
This updated patch just ensures the included script runs from make check in the
regressions directory.
10:06 a.m.
New subject: [PATCH] Update list-filesystems to check md devices
---
regressions/Makefile.am | 1 +
regressions/test-list-filesystems.sh | 77 ++++++++++++++++++++++++++++++++++
src/listfs.c | 60 ++++++++++++++-------------
3 files changed, 109 insertions(+), 29 deletions(-)
create mode 100755 regressions/test-list-filesystems.sh
8:53 a.m.
On Thu, Nov 10, 2011 at 02:19:11PM +0000, Matthew Booth wrote:
+ /* Look for directories under /sys/block matching md[0-9]*
+ * As an additional check, we also make sure they have a md subdirectory */
+ int err = glob(PREFIX "[0-9]*" SUFFIX, GLOB_ERR, NULL, &mds);
+ if (err == GLOB_NOSPACE) {
+ reply_with_error("OOM searching for md devices");
+ goto error;
+ } else if (err == GLOB_ABORTED) {
+ reply_with_error("Read error searching for md devices");
+ goto error;
Since glob is supposed to set errno on error, I'd prefer:
reply_with_perror ("glob");
for both of the branches above (combine them into a single branch
if you like).
+ } else if (err == GLOB_NOMATCH) {
+ /* This is fine */
+ }
I think this branch should just be dropped.
+ for (size_t i = 0; i < mds.gl_pathc; i++) {
+ size_t len = strlen(mds.gl_pathv[i]) - strlen(PREFIX) - strlen(SUFFIX);
+
+#define DEV "/dev/md"
+ char *dev = malloc(strlen(DEV) + len + 1);
Need to check the return value from malloc and reply_with_perror /
goto error on failure.
+# Create a raid1 based on the 2 disks
+debug sh "/usr/bin/yes | mdadm -C /dev/md/test --level=1 --raid-devices=2 /dev/vda
/dev/vdb >/dev/null 2>&1"
I guessed we'd be running the debug backdoor here to create devices.
Is it not worth binding this call too?
+# Ensure list-md-devices now returns the newly create md device
"created"
Generally looking good.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
9:07 a.m.
On 10/11/11 14:53, Richard W.M. Jones wrote:
On Thu, Nov 10, 2011 at 02:19:11PM +0000, Matthew Booth wrote:
> + /* Look for directories under /sys/block matching md[0-9]*
> + * As an additional check, we also make sure they have a md subdirectory */
> + int err = glob(PREFIX "[0-9]*" SUFFIX, GLOB_ERR, NULL,&mds);
> + if (err == GLOB_NOSPACE) {
> + reply_with_error("OOM searching for md devices");
> + goto error;
> + } else if (err == GLOB_ABORTED) {
> + reply_with_error("Read error searching for md devices");
> + goto error;
Since glob is supposed to set errno on error, I'd prefer:
reply_with_perror ("glob");
for both of the branches above (combine them into a single branch
if you like).
glob has a callback (which I'm not using) for returning an error for
each individual failure it hits. It's not actually documented to set
errno at all. A glob failure here is pretty unlikely, so writing a bunch
of code to collate errors and report them all nicely seems a waste.
Simply catching an error seems good enough.
> + } else if (err == GLOB_NOMATCH) {
> + /* This is fine */
> + }
I think this branch should just be dropped.
Heh, yeah. It used to have code in it :)
> + for (size_t i = 0; i< mds.gl_pathc; i++) {
> + size_t len = strlen(mds.gl_pathv[i]) - strlen(PREFIX) - strlen(SUFFIX);
> +
> +#define DEV "/dev/md"
> + char *dev = malloc(strlen(DEV) + len + 1);
Need to check the return value from malloc and reply_with_perror /
goto error on failure.
Ok.
> +# Create a raid1 based on the 2 disks
> +debug sh "/usr/bin/yes | mdadm -C /dev/md/test --level=1 --raid-devices=2
/dev/vda /dev/vdb>/dev/null 2>&1"
I guessed we'd be running the debug backdoor here to create devices.
Is it not worth binding this call too?
Maybe another day :)
> +# Ensure list-md-devices now returns the newly create md device
"created"
Generally looking good.
Updated patch to follow.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
9:07 a.m.
On Thu, Nov 10, 2011 at 02:53:48PM +0000, Richard W.M. Jones wrote:
On Thu, Nov 10, 2011 at 02:19:11PM +0000, Matthew Booth wrote:
> + /* Look for directories under /sys/block matching md[0-9]*
> + * As an additional check, we also make sure they have a md subdirectory */
> + int err = glob(PREFIX "[0-9]*" SUFFIX, GLOB_ERR, NULL, &mds);
> + if (err == GLOB_NOSPACE) {
> + reply_with_error("OOM searching for md devices");
> + goto error;
> + } else if (err == GLOB_ABORTED) {
> + reply_with_error("Read error searching for md devices");
> + goto error;
Since glob is supposed to set errno on error, I'd prefer:
reply_with_perror ("glob");
for both of the branches above (combine them into a single branch
if you like).
Apparently glob doesn't necessarily set errno. Can we capture the
errno and at least dump the message out to stderr? A simple callback
that just calls perror [not reply_with_perror] should be enough.
+ if (err == GLOB_NOSPACE) {
+ reply_with_error("OOM searching for md devices");
+ goto error;
+ } else if (err == GLOB_ABORTED) {
+ reply_with_error("Read error searching for md devices");
+ goto error;
These error messages are still fairly grotty. It would be some
consistent to have something like this:
+ if (err == GLOB_NOSPACE) {
+ reply_with_error ("glob: returned GLOB_NOSPACE: rerun with
LIBGUESTFS_DEBUG=1");
+ goto error;
+ } else if (err == GLOB_ABORTED) {
+ reply_with_error ("glob: returned GLOB_ABORTED: rerun with
LIBGUESTFS_DEBUG=1");
+ goto error;
That should be enough for us to go and track the problem back to the
code if it ever did occur.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
9:53 a.m.
New subject: [PATCH] New API: list_md_devices
Return a list of Linux MD devices detected in the guest.
This API complements list_devices, list_partitions, list_lvs and
list_dm_devices.
---
daemon/Makefile.am | 1 +
daemon/daemon.h | 1 +
daemon/guestfsd.c | 23 ++++++---
daemon/md.c | 88 +++++++++++++++++++++++++++++++++++
generator/generator_actions.ml | 10 ++++
po/POTFILES.in | 1 +
regressions/Makefile.am | 1 +
regressions/test-list-md-devices.sh | 59 +++++++++++++++++++++++
src/MAX_PROC_NR | 2 +-
9 files changed, 178 insertions(+), 8 deletions(-)
create mode 100644 daemon/md.c
create mode 100755 regressions/test-list-md-devices.sh
12:50 p.m.
New subject: [PATCH] New API: list_md_devices
I may be missing some part of this patch, but I cannot see any md
devices to be listed under /dev, and list-md-devices always returns an
empty list.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
3:36 a.m.
New subject: [PATCH] New API: list_md_devices
On 10/11/11 18:50, Richard W.M. Jones wrote:
I may be missing some part of this patch, but I cannot see any md
devices to be listed under /dev, and list-md-devices always returns an
empty list.
Rich.
This patch has no bearing on whether the appliance detects md devices or
not. That was enabled in a previous patch by including mdadm in the
appliance.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
4759
days inactive
4760
days old
10 comments
2 participants
participants (2)
-
Matthew Booth -
Richard W.M. Jones