Libguestfs - Sep 2019 - [libnbd PATCH] api: Add set_handshake_flags for integration

Similar to the recent --mask-handshake command line added to nbdkit to
test client fallbacks to crippled servers, it can be worth testing
server fallbacks to crippled clients.  And just as we have exposed
whether the client will request structured replies, we can also expose
whether the client will understand various handshake flags from the
NBD protocol.

Of course, we default to supporting all flags that we understand and
which are advertised by the server.  But clearing FIXED_NEWSTYLE lets
us test NBD_OPT_EXPORT_NAME handling, and clearing NO_ZEROES lets us
test whether the zero padding in response to NBD_OPT_EXPORT_NAME is
correct.
---

I'm still considering the addition of tests/* against nbd, or interop/*
against qemu, to ensure that we have interoperability between various
degraded connection modes. But that can be a separate patch.

 lib/internal.h                              |  1 +
 lib/nbd-protocol.h                          |  5 +-
 generator/generator                         | 72 ++++++++++++++++++++-
 generator/states-newstyle-opt-export-name.c |  2 +-
 generator/states-newstyle.c                 | 14 ++--
 generator/states-oldstyle.c                 |  5 ++
 lib/handle.c                                | 18 ++++++
 7 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index ccaca32..998ca3d 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -157,6 +157,7 @@ struct nbd_handle {
         } __attribute__((packed)) error;
       } payload;
     }  __attribute__((packed)) sr;
+    uint16_t gflags;
     uint32_t cflags;
     uint32_t len;
     uint16_t nrinfos;
diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
index 04e93d3..699aa22 100644
--- a/lib/nbd-protocol.h
+++ b/lib/nbd-protocol.h
@@ -93,9 +93,10 @@ struct nbd_fixed_new_option_reply {

 #define NBD_REP_MAGIC UINT64_C(0x3e889045565a9)

-/* Global flags. */
+/* Global flags. Exposed by the generator as LIBNBD_HANDSHAKE_FLAG_* instead
 #define NBD_FLAG_FIXED_NEWSTYLE 1
 #define NBD_FLAG_NO_ZEROES      2
+ */

 /* Per-export flags. */
 #define NBD_FLAG_HAS_FLAGS         (1 << 0)
@@ -238,10 +239,12 @@ struct nbd_structured_reply_error {
 #define NBD_CMD_WRITE_ZEROES      6
 #define NBD_CMD_BLOCK_STATUS      7

+/* Command flags. Exposed by the generator as LIBNBD_CMD_FLAG_* instead
 #define NBD_CMD_FLAG_FUA      (1<<0)
 #define NBD_CMD_FLAG_NO_HOLE  (1<<1)
 #define NBD_CMD_FLAG_DF       (1<<2)
 #define NBD_CMD_FLAG_REQ_ONE  (1<<3)
+*/

 /* NBD error codes. */
 #define NBD_SUCCESS     0
diff --git a/generator/generator b/generator/generator
index a72f36c..170121e 100755
--- a/generator/generator
+++ b/generator/generator
@@ -971,7 +971,14 @@ let cmd_flags = {
     "FAST_ZERO", 1 lsl 4;
   ]
 }
-let all_flags = [ cmd_flags ]
+let handshake_flags = {
+  flag_prefix = "HANDSHAKE_FLAG";
+  flags = [
+    "FIXED_NEWSTYLE", 1 lsl 0;
+    "NO_ZEROES",      1 lsl 1;
+    ]
+}
+let all_flags = [ cmd_flags; handshake_flags ]

 (* Calls.
  *
@@ -1261,6 +1268,7 @@ for integration testing, it can be useful to clear this
flag
 rather than find a way to alter the server to fail the negotiation
 request.";
     see_also = ["L<nbd_get_request_structured_replies(3)>";
+                "L<nbd_set_handshake_flags(3)>";
                 "L<nbd_can_meta_context(3)>";
"L<nbd_can_df(3)>"];
   };

@@ -1277,6 +1285,66 @@ able to honor that request";
     see_also = ["L<nbd_set_request_structured_replies(3)>"];
   };

+  "set_handshake_flags", {
+    default_call with
+    args = [ Flags ("flags", handshake_flags) ]; ret = RErr;
+    permitted_states = [ Created ];
+    shortdesc = "control use of handshake flags";
+    longdesc = "\
+By default, libnbd tries to negotiate all possible handshake flags
+that are also supported by the server; since omitting a handshake
+flag can prevent the use of other functionality such as TLS encryption
+or structured replies.  However, for integration testing, it can be
+useful to reduce the set of flags supported by the client to test that
+a particular server can handle various clients that were compliant to
+older versions of the NBD specification.
+
+The C<flags> argument is a bitmask, including zero or more of the
+following handshake flags:
+
+=over 4
+
+=item C<LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE> = 1
+
+The server gracefully handles unknown option requests from the
+client, rather than disconnecting.  Without this flag, a client
+cannot safely request to use extensions such as TLS encryption or
+structured replies, as the request may cause an older server to
+drop the connection.
+
+=item C<LIBNBD_HANDSHAKE_FLAG_NO_ZEROES> = 2
+
+If the client is forced to use C<NBD_OPT_EXPORT_NAME> instead of
+the preferred C<NBD_OPT_GO>, this flag allows the server to send
+fewer all-zero padding bytes over the connection.
+
+=back
+
+Future NBD extensions may add further flags.
+";
+    see_also = ["L<nbd_get_handshake_flags(3)>";
+                "L<nbd_set_request_structured_replies(3)>"];
+  };
+
+  "get_handshake_flags", {
+    default_call with
+    args = []; ret = RUInt;
+    may_set_error = false;
+    shortdesc = "see which handshake flags are supported";
+    longdesc = "\
+Return the state of the handshake flags on this handle.  When the
+handle has not yet completed a connection (see C<nbd_aio_is_created>),
+this returns the flags that the client is willing to use, provided
+the server also advertises those flags.  After the connection is
+ready (see C<nbd_aio_is_ready>), this returns the flags that were
+actually agreed on between the server and client.  If the NBD
+protocol defines new handshake flags, then the return value from
+a newer library version may include bits that were undefined at
+the time of compilation.";
+    see_also = ["L<nbd_set_handshake_flags(3)>";
+                "L<nbd_aio_is_created(3)>";
"L<nbd_aio_is_ready(3)>"];
+  };
+
   "add_meta_context", {
     default_call with
     args = [ String "name" ]; ret = RErr;
@@ -2521,6 +2589,8 @@ let first_version = [
   "can_fast_zero", (1, 2);
   "set_request_structured_replies", (1, 2);
   "get_request_structured_replies", (1, 2);
+  "set_handshake_flags", (1, 2);
+  "get_handshake_flags", (1, 2);

   (* These calls are proposed for a future version of libnbd, but
    * have not been added to any released version so far.
diff --git a/generator/states-newstyle-opt-export-name.c
b/generator/states-newstyle-opt-export-name.c
index ec73136..a46d851 100644
--- a/generator/states-newstyle-opt-export-name.c
+++ b/generator/states-newstyle-opt-export-name.c
@@ -45,7 +45,7 @@
   case 0:
     h->rbuf = &h->sbuf;
     h->rlen = sizeof h->sbuf.export_name_reply;
-    if ((h->gflags & NBD_FLAG_NO_ZEROES) != 0)
+    if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_NO_ZEROES) != 0)
       h->rlen -= sizeof h->sbuf.export_name_reply.zeroes;
     SET_NEXT_STATE (%RECV_REPLY);
   }
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index b4f2b80..8c3ae8a 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -112,8 +112,8 @@ handle_reply_error (struct nbd_handle *h)

 /* STATE MACHINE */ {
  NEWSTYLE.START:
-  h->rbuf = &h->gflags;
-  h->rlen = 2;
+  h->rbuf = &h->sbuf;
+  h->rlen = sizeof h->sbuf.gflags;
   SET_NEXT_STATE (%RECV_GFLAGS);
   return 0;

@@ -127,16 +127,16 @@ handle_reply_error (struct nbd_handle *h)
  NEWSTYLE.CHECK_GFLAGS:
   uint32_t cflags;

-  h->gflags = be16toh (h->gflags);
-  if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0 &&
+  h->gflags &= be16toh (h->sbuf.gflags);
+  if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0 &&
       h->tls == LIBNBD_TLS_REQUIRE) {
     SET_NEXT_STATE (%.DEAD);
-    set_error (ENOTSUP, "handshake: server is not fixed newstyle, "
+    set_error (ENOTSUP, "handshake: server is not using fixed newstyle,
"
                "but handle TLS setting is 'require' (2)");
     return 0;
   }

-  cflags = h->gflags & (NBD_FLAG_FIXED_NEWSTYLE|NBD_FLAG_NO_ZEROES);
+  cflags = h->gflags;
   h->sbuf.cflags = htobe32 (cflags);
   h->wbuf = &h->sbuf;
   h->wlen = 4;
@@ -148,7 +148,7 @@ handle_reply_error (struct nbd_handle *h)
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
     /* Start sending options. */
-    if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0)
+    if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
       SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
     else
       SET_NEXT_STATE (%OPT_STARTTLS.START);
diff --git a/generator/states-oldstyle.c b/generator/states-oldstyle.c
index babefc0..c0d8af8 100644
--- a/generator/states-oldstyle.c
+++ b/generator/states-oldstyle.c
@@ -58,6 +58,11 @@

   h->gflags = gflags;
   debug (h, "gflags: 0x%" PRIx16, gflags);
+  if (gflags) {
+    set_error (0, "handshake: oldstyle server should not set
gflags");
+    SET_NEXT_STATE (%.DEAD);
+    return 0;
+  }

   if (nbd_internal_set_size_and_flags (h, exportsize, eflags) == -1) {
     SET_NEXT_STATE (%.DEAD);
diff --git a/lib/handle.c b/lib/handle.c
index bc4206c..8ca2e5a 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -64,6 +64,8 @@ nbd_create (void)
   h->unique = 1;
   h->tls_verify_peer = true;
   h->request_sr = true;
+  h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
+               LIBNBD_HANDSHAKE_FLAG_NO_ZEROES);

   s = getenv ("LIBNBD_DEBUG");
   h->debug = s && strcmp (s, "1") == 0;
@@ -258,6 +260,22 @@ nbd_unlocked_get_request_structured_replies (struct
nbd_handle *h)
   return h->request_sr;
 }

+int
+nbd_unlocked_set_handshake_flags (struct nbd_handle *h,
+                                  uint32_t flags)
+{
+  /* The generator already ensured flags was in range. */
+  h->gflags = flags;
+  return 0;
+}
+
+/* NB: may_set_error = false. */
+uint32_t
+nbd_unlocked_get_handshake_flags (struct nbd_handle *h)
+{
+  return h->gflags;
+}
+
 const char *
 nbd_unlocked_get_package_name (struct nbd_handle *h)
 {
-- 
2.21.0

On Mon, Sep 16, 2019 at 02:35:33PM -0500, Eric Blake
wrote:
> Similar to the recent --mask-handshake command line added to nbdkit to
> test client fallbacks to crippled servers, it can be worth testing
> server fallbacks to crippled clients.  And just as we have exposed
> whether the client will request structured replies, we can also expose
> whether the client will understand various handshake flags from the
> NBD protocol.
> 
> Of course, we default to supporting all flags that we understand and
> which are advertised by the server.  But clearing FIXED_NEWSTYLE lets
> us test NBD_OPT_EXPORT_NAME handling, and clearing NO_ZEROES lets us
> test whether the zero padding in response to NBD_OPT_EXPORT_NAME is
> correct.
> ---
> 
> I'm still considering the addition of tests/* against nbd, or interop/*
> against qemu, to ensure that we have interoperability between various
> degraded connection modes. But that can be a separate patch.
> 
>  lib/internal.h                              |  1 +
>  lib/nbd-protocol.h                          |  5 +-
>  generator/generator                         | 72 ++++++++++++++++++++-
>  generator/states-newstyle-opt-export-name.c |  2 +-
>  generator/states-newstyle.c                 | 14 ++--
>  generator/states-oldstyle.c                 |  5 ++
>  lib/handle.c                                | 18 ++++++
>  7 files changed, 107 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/internal.h b/lib/internal.h
> index ccaca32..998ca3d 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -157,6 +157,7 @@ struct nbd_handle {
>          } __attribute__((packed)) error;
>        } payload;
>      }  __attribute__((packed)) sr;
> +    uint16_t gflags;
>      uint32_t cflags;
>      uint32_t len;
>      uint16_t nrinfos;
> diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
> index 04e93d3..699aa22 100644
> --- a/lib/nbd-protocol.h
> +++ b/lib/nbd-protocol.h
> @@ -93,9 +93,10 @@ struct nbd_fixed_new_option_reply {
> 
>  #define NBD_REP_MAGIC UINT64_C(0x3e889045565a9)
> 
> -/* Global flags. */
> +/* Global flags. Exposed by the generator as LIBNBD_HANDSHAKE_FLAG_*
instead
>  #define NBD_FLAG_FIXED_NEWSTYLE 1
>  #define NBD_FLAG_NO_ZEROES      2
> + */
> 
>  /* Per-export flags. */
>  #define NBD_FLAG_HAS_FLAGS         (1 << 0)
> @@ -238,10 +239,12 @@ struct nbd_structured_reply_error {
>  #define NBD_CMD_WRITE_ZEROES      6
>  #define NBD_CMD_BLOCK_STATUS      7
> 
> +/* Command flags. Exposed by the generator as LIBNBD_CMD_FLAG_* instead
>  #define NBD_CMD_FLAG_FUA      (1<<0)
>  #define NBD_CMD_FLAG_NO_HOLE  (1<<1)
>  #define NBD_CMD_FLAG_DF       (1<<2)
>  #define NBD_CMD_FLAG_REQ_ONE  (1<<3)
> +*/
> 
>  /* NBD error codes. */
>  #define NBD_SUCCESS     0
> diff --git a/generator/generator b/generator/generator
> index a72f36c..170121e 100755
> --- a/generator/generator
> +++ b/generator/generator
> @@ -971,7 +971,14 @@ let cmd_flags = {
>      "FAST_ZERO", 1 lsl 4;
>    ]
>  }
> -let all_flags = [ cmd_flags ]
> +let handshake_flags = {
> +  flag_prefix = "HANDSHAKE_FLAG";
> +  flags = [
> +    "FIXED_NEWSTYLE", 1 lsl 0;
> +    "NO_ZEROES",      1 lsl 1;
> +    ]
> +}
> +let all_flags = [ cmd_flags; handshake_flags ]
> 
>  (* Calls.
>   *
> @@ -1261,6 +1268,7 @@ for integration testing, it can be useful to clear
this flag
>  rather than find a way to alter the server to fail the negotiation
>  request.";
>      see_also =
["L<nbd_get_request_structured_replies(3)>";
> +                "L<nbd_set_handshake_flags(3)>";
>                  "L<nbd_can_meta_context(3)>";
"L<nbd_can_df(3)>"];
>    };
> 
> @@ -1277,6 +1285,66 @@ able to honor that request";
>      see_also =
["L<nbd_set_request_structured_replies(3)>"];
>    };
> 
> +  "set_handshake_flags", {
> +    default_call with
> +    args = [ Flags ("flags", handshake_flags) ]; ret = RErr;
> +    permitted_states = [ Created ];
> +    shortdesc = "control use of handshake flags";
> +    longdesc = "\
> +By default, libnbd tries to negotiate all possible handshake flags
> +that are also supported by the server; since omitting a handshake
> +flag can prevent the use of other functionality such as TLS encryption
> +or structured replies.  However, for integration testing, it can be
> +useful to reduce the set of flags supported by the client to test that
> +a particular server can handle various clients that were compliant to
> +older versions of the NBD specification.
> +
> +The C<flags> argument is a bitmask, including zero or more of the
> +following handshake flags:
> +
> +=over 4
> +
> +=item C<LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE> = 1
> +
> +The server gracefully handles unknown option requests from the
> +client, rather than disconnecting.  Without this flag, a client
> +cannot safely request to use extensions such as TLS encryption or
> +structured replies, as the request may cause an older server to
> +drop the connection.
> +
> +=item C<LIBNBD_HANDSHAKE_FLAG_NO_ZEROES> = 2
> +
> +If the client is forced to use C<NBD_OPT_EXPORT_NAME> instead of
> +the preferred C<NBD_OPT_GO>, this flag allows the server to send
> +fewer all-zero padding bytes over the connection.
> +
> +=back
> +
> +Future NBD extensions may add further flags.
> +";
> +    see_also = ["L<nbd_get_handshake_flags(3)>";
> +               
"L<nbd_set_request_structured_replies(3)>"];
> +  };
> +
> +  "get_handshake_flags", {
> +    default_call with
> +    args = []; ret = RUInt;
> +    may_set_error = false;
> +    shortdesc = "see which handshake flags are supported";
> +    longdesc = "\
> +Return the state of the handshake flags on this handle.  When the
> +handle has not yet completed a connection (see
C<nbd_aio_is_created>),
> +this returns the flags that the client is willing to use, provided
> +the server also advertises those flags.  After the connection is
> +ready (see C<nbd_aio_is_ready>), this returns the flags that were
> +actually agreed on between the server and client.  If the NBD
> +protocol defines new handshake flags, then the return value from
> +a newer library version may include bits that were undefined at
> +the time of compilation.";
> +    see_also = ["L<nbd_set_handshake_flags(3)>";
> +                "L<nbd_aio_is_created(3)>";
"L<nbd_aio_is_ready(3)>"];
> +  };
> +
>    "add_meta_context", {
>      default_call with
>      args = [ String "name" ]; ret = RErr;
> @@ -2521,6 +2589,8 @@ let first_version = [
>    "can_fast_zero", (1, 2);
>    "set_request_structured_replies", (1, 2);
>    "get_request_structured_replies", (1, 2);
> +  "set_handshake_flags", (1, 2);
> +  "get_handshake_flags", (1, 2);
> 
>    (* These calls are proposed for a future version of libnbd, but
>     * have not been added to any released version so far.
> diff --git a/generator/states-newstyle-opt-export-name.c
b/generator/states-newstyle-opt-export-name.c
> index ec73136..a46d851 100644
> --- a/generator/states-newstyle-opt-export-name.c
> +++ b/generator/states-newstyle-opt-export-name.c
> @@ -45,7 +45,7 @@
>    case 0:
>      h->rbuf = &h->sbuf;
>      h->rlen = sizeof h->sbuf.export_name_reply;
> -    if ((h->gflags & NBD_FLAG_NO_ZEROES) != 0)
> +    if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_NO_ZEROES) != 0)
>        h->rlen -= sizeof h->sbuf.export_name_reply.zeroes;
>      SET_NEXT_STATE (%RECV_REPLY);
>    }
> diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
> index b4f2b80..8c3ae8a 100644
> --- a/generator/states-newstyle.c
> +++ b/generator/states-newstyle.c
> @@ -112,8 +112,8 @@ handle_reply_error (struct nbd_handle *h)
> 
>  /* STATE MACHINE */ {
>   NEWSTYLE.START:
> -  h->rbuf = &h->gflags;
> -  h->rlen = 2;
> +  h->rbuf = &h->sbuf;
> +  h->rlen = sizeof h->sbuf.gflags;
>    SET_NEXT_STATE (%RECV_GFLAGS);
>    return 0;
> 
> @@ -127,16 +127,16 @@ handle_reply_error (struct nbd_handle *h)
>   NEWSTYLE.CHECK_GFLAGS:
>    uint32_t cflags;
> 
> -  h->gflags = be16toh (h->gflags);
> -  if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0 &&
> +  h->gflags &= be16toh (h->sbuf.gflags);
> +  if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0
&&
>        h->tls == LIBNBD_TLS_REQUIRE) {
>      SET_NEXT_STATE (%.DEAD);
> -    set_error (ENOTSUP, "handshake: server is not fixed newstyle,
"
> +    set_error (ENOTSUP, "handshake: server is not using fixed
newstyle, "
>                 "but handle TLS setting is 'require'
(2)");
>      return 0;
>    }
> 
> -  cflags = h->gflags &
(NBD_FLAG_FIXED_NEWSTYLE|NBD_FLAG_NO_ZEROES);
> +  cflags = h->gflags;
>    h->sbuf.cflags = htobe32 (cflags);
>    h->wbuf = &h->sbuf;
>    h->wlen = 4;
> @@ -148,7 +148,7 @@ handle_reply_error (struct nbd_handle *h)
>    case -1: SET_NEXT_STATE (%.DEAD); return 0;
>    case 0:
>      /* Start sending options. */
> -    if ((h->gflags & NBD_FLAG_FIXED_NEWSTYLE) == 0)
> +    if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0)
>        SET_NEXT_STATE (%OPT_EXPORT_NAME.START);
>      else
>        SET_NEXT_STATE (%OPT_STARTTLS.START);
> diff --git a/generator/states-oldstyle.c b/generator/states-oldstyle.c
> index babefc0..c0d8af8 100644
> --- a/generator/states-oldstyle.c
> +++ b/generator/states-oldstyle.c
> @@ -58,6 +58,11 @@
> 
>    h->gflags = gflags;
>    debug (h, "gflags: 0x%" PRIx16, gflags);
> +  if (gflags) {
> +    set_error (0, "handshake: oldstyle server should not set
gflags");
> +    SET_NEXT_STATE (%.DEAD);
> +    return 0;
> +  }
> 
>    if (nbd_internal_set_size_and_flags (h, exportsize, eflags) == -1) {
>      SET_NEXT_STATE (%.DEAD);
> diff --git a/lib/handle.c b/lib/handle.c
> index bc4206c..8ca2e5a 100644
> --- a/lib/handle.c
> +++ b/lib/handle.c
> @@ -64,6 +64,8 @@ nbd_create (void)
>    h->unique = 1;
>    h->tls_verify_peer = true;
>    h->request_sr = true;
> +  h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
> +               LIBNBD_HANDSHAKE_FLAG_NO_ZEROES);
> 
>    s = getenv ("LIBNBD_DEBUG");
>    h->debug = s && strcmp (s, "1") == 0;
> @@ -258,6 +260,22 @@ nbd_unlocked_get_request_structured_replies (struct
nbd_handle *h)
>    return h->request_sr;
>  }
> 
> +int
> +nbd_unlocked_set_handshake_flags (struct nbd_handle *h,
> +                                  uint32_t flags)
> +{
> +  /* The generator already ensured flags was in range. */
> +  h->gflags = flags;
> +  return 0;
> +}
> +
> +/* NB: may_set_error = false. */
> +uint32_t
> +nbd_unlocked_get_handshake_flags (struct nbd_handle *h)
> +{
> +  return h->gflags;
> +}
> +
>  const char *
>  nbd_unlocked_get_package_name (struct nbd_handle *h)
>  {
As you say it could do with a test or interop test, but:

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/