[Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Vladimir Sementsov-Ogievskiy]

It's unexpected behavior that without -x option qemu-nbd do old-style
negotiation. Let's use "" as a default name instead (as it is already
done if tls is used) and therefore, drop old-style negotiation from
Qemu NBD server.

Vladimir Sementsov-Ogievskiy (2):
  qemu-nbd: drop old-style negotiation
  nbd/server: drop old-style negotiation

 include/block/nbd.h |  3 +--
 blockdev-nbd.c      |  2 +-
 nbd/server.c        | 53 +++++++++++++--------------------------------
 qemu-nbd.c          | 25 +++++----------------
 4 files changed, 23 insertions(+), 60 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH 1/2] qemu-nbd: drop old-style negotiation

[Vladimir Sementsov-Ogievskiy]

Use new-style negotiation always, with default "" (empty) export name
if it is not specified with '-x' option.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-nbd.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b9d38c72..57e79e30ea 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -56,7 +56,6 @@
 #define MBR_SIZE 512
 
 static NBDExport *exp;
-static bool newproto;
 static int verbose;
 static char *srcpath;
 static SocketAddress *saddr;
@@ -84,8 +83,8 @@ static void usage(const char *name)
 "  -e, --shared=NUM          device can be shared by NUM clients (default '1')\n"
 "  -t, --persistent          don't exit on the last connection\n"
 "  -v, --verbose             display extra debugging information\n"
-"  -x, --export-name=NAME    expose export by name\n"
-"  -D, --description=TEXT    with -x, also export a human-readable description\n"
+"  -x, --export-name=NAME    expose export by name (default is empty string)\n"
+"  -D, --description=TEXT    export a human-readable description\n"
 "\n"
 "Exposing part of the image:\n"
 "  -o, --offset=OFFSET       offset into the image\n"
@@ -354,8 +353,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
 
     nb_fds++;
     nbd_update_server_watch();
-    nbd_client_new(newproto ? NULL : exp, cioc,
-                   tlscreds, NULL, nbd_client_closed);
+    nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed);
 }
 
 static void nbd_update_server_watch(void)
@@ -549,7 +547,7 @@ int main(int argc, char **argv)
     Error *local_err = NULL;
     BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
     QDict *options = NULL;
-    const char *export_name = NULL;
+    const char *export_name = ""; /* Default export name */
     const char *export_description = NULL;
     const char *tlscredsid = NULL;
     bool imageOpts = false;
@@ -808,11 +806,6 @@ int main(int argc, char **argv)
             error_report("TLS is not supported with a host device");
             exit(EXIT_FAILURE);
         }
-        if (!export_name) {
-            /* Set the default NBD protocol export name, since
-             * we *must* use new style protocol for TLS */
-            export_name = "";
-        }
         tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
         if (local_err) {
             error_report("Failed to get TLS creds %s",
@@ -1013,14 +1006,8 @@ int main(int argc, char **argv)
         error_report_err(local_err);
         exit(EXIT_FAILURE);
     }
-    if (export_name) {
-        nbd_export_set_name(exp, export_name);
-        nbd_export_set_description(exp, export_description);
-        newproto = true;
-    } else if (export_description) {
-        error_report("Export description requires an export name");
-        exit(EXIT_FAILURE);
-    }
+    nbd_export_set_name(exp, export_name);
+    nbd_export_set_description(exp, export_description);
 
     if (device) {
         int ret;
-- 
2.18.0

[Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation

[Vladimir Sementsov-Ogievskiy]

After the previous commit, nbd_client_new first parameter is always
NULL. Let's drop it with all corresponding old-style negotiation code
path which is unreachable now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/nbd.h |  3 +--
 blockdev-nbd.c      |  2 +-
 nbd/server.c        | 53 +++++++++++++--------------------------------
 qemu-nbd.c          |  2 +-
 4 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 4638c839f5..0129d1a4b4 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -308,8 +308,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name);
 void nbd_export_set_description(NBDExport *exp, const char *description);
 void nbd_export_close_all(void);
 
-void nbd_client_new(NBDExport *exp,
-                    QIOChannelSocket *sioc,
+void nbd_client_new(QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
                     void (*close_fn)(NBDClient *, bool));
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 1ef11041a7..8913d8ff73 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -36,7 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
                        gpointer opaque)
 {
     qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
-    nbd_client_new(NULL, cioc,
+    nbd_client_new(cioc,
                    nbd_server->tlscreds, NULL,
                    nbd_blockdev_client_closed);
 }
diff --git a/nbd/server.c b/nbd/server.c
index 1fba21414b..e87f881525 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1253,7 +1253,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
     const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
                               NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
                               NBD_FLAG_SEND_WRITE_ZEROES | NBD_FLAG_SEND_CACHE);
-    bool oldStyle;
 
     /* Old style negotiation header, no room for options
         [ 0 ..   7]   passwd       ("NBDMAGIC")
@@ -1274,33 +1273,19 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
     trace_nbd_negotiate_begin();
     memcpy(buf, "NBDMAGIC", 8);
 
-    oldStyle = client->exp != NULL && !client->tlscreds;
-    if (oldStyle) {
-        trace_nbd_negotiate_old_style(client->exp->size,
-                                      client->exp->nbdflags | myflags);
-        stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
-        stq_be_p(buf + 16, client->exp->size);
-        stl_be_p(buf + 24, client->exp->nbdflags | myflags);
+    stq_be_p(buf + 8, NBD_OPTS_MAGIC);
+    stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
 
-        if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
-            error_prepend(errp, "write failed: ");
-            return -EINVAL;
-        }
-    } else {
-        stq_be_p(buf + 8, NBD_OPTS_MAGIC);
-        stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
-
-        if (nbd_write(client->ioc, buf, 18, errp) < 0) {
-            error_prepend(errp, "write failed: ");
-            return -EINVAL;
-        }
-        ret = nbd_negotiate_options(client, myflags, errp);
-        if (ret != 0) {
-            if (ret < 0) {
-                error_prepend(errp, "option negotiation failed: ");
-            }
-            return ret;
+    if (nbd_write(client->ioc, buf, 18, errp) < 0) {
+        error_prepend(errp, "write failed: ");
+        return -EINVAL;
+    }
+    ret = nbd_negotiate_options(client, myflags, errp);
+    if (ret != 0) {
+        if (ret < 0) {
+            error_prepend(errp, "option negotiation failed: ");
         }
+        return ret;
     }
 
     assert(!client->optlen);
@@ -2396,13 +2381,8 @@ static void nbd_client_receive_next_request(NBDClient *client)
 static coroutine_fn void nbd_co_client_start(void *opaque)
 {
     NBDClient *client = opaque;
-    NBDExport *exp = client->exp;
     Error *local_err = NULL;
 
-    if (exp) {
-        nbd_export_get(exp);
-        QTAILQ_INSERT_TAIL(&exp->clients, client, next);
-    }
     qemu_co_mutex_init(&client->send_lock);
 
     if (nbd_negotiate(client, &local_err)) {
@@ -2417,13 +2397,11 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
 }
 
 /*
- * Create a new client listener on the given export @exp, using the
- * given channel @sioc.  Begin servicing it in a coroutine.  When the
- * connection closes, call @close_fn with an indication of whether the
- * client completed negotiation.
+ * Create a new client listener using the given channel @sioc.
+ * Begin servicing it in a coroutine.  When the connection closes, call
+ * @close_fn with an indication of whether the client completed negotiation.
  */
-void nbd_client_new(NBDExport *exp,
-                    QIOChannelSocket *sioc,
+void nbd_client_new(QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
                     void (*close_fn)(NBDClient *, bool))
@@ -2433,7 +2411,6 @@ void nbd_client_new(NBDExport *exp,
 
     client = g_new0(NBDClient, 1);
     client->refcount = 1;
-    client->exp = exp;
     client->tlscreds = tlscreds;
     if (tlscreds) {
         object_ref(OBJECT(client->tlscreds));
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 57e79e30ea..aad0b88723 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -353,7 +353,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
 
     nb_fds++;
     nbd_update_server_watch();
-    nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed);
+    nbd_client_new(cioc, tlscreds, NULL, nbd_client_closed);
 }
 
 static void nbd_update_server_watch(void)
-- 
2.18.0

Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: drop old-style negotiation

[Eric Blake]

On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> Use new-style negotiation always, with default "" (empty) export name
> if it is not specified with '-x' option.

If we like this approach (over mine of keeping oldstyle, but via an 
explicit -O option), then this commit message should add the following:

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   qemu-nbd.c | 25 ++++++-------------------
>   1 file changed, 6 insertions(+), 19 deletions(-)
> 

If we like your patch over mine,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Eric Blake]

On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> It's unexpected behavior that without -x option qemu-nbd do old-style
> negotiation. Let's use "" as a default name instead (as it is already
> done if tls is used) and therefore, drop old-style negotiation from
> Qemu NBD server.

Oddly enough, I wrote a similar patch in parallel, and am only now just 
seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd 
-O' to allow explicit fallback to oldstyle, while you ripped it out 
altogether).  The client can negotiate either style, so we don't need an 
option on the client side; rather, this is all about what the server 
should do by default.

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html

Does anyone have a preference between the two? Here's the last time it 
was discussed:

https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html

> 
> Vladimir Sementsov-Ogievskiy (2):
>    qemu-nbd: drop old-style negotiation
>    nbd/server: drop old-style negotiation
> 
>   include/block/nbd.h |  3 +--
>   blockdev-nbd.c      |  2 +-
>   nbd/server.c        | 53 +++++++++++++--------------------------------
>   qemu-nbd.c          | 25 +++++----------------
>   4 files changed, 23 insertions(+), 60 deletions(-)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation

[Eric Blake]

On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> After the previous commit, nbd_client_new first parameter is always
> NULL. Let's drop it with all corresponding old-style negotiation code
> path which is unreachable now.

Being able to force oldstyle negotiation for interoperability testing 
may still be useful. But as fewer and fewer interesting clients exist 
that want oldstyle (after all, extensions are only usable with 
newstyle), I'm finding it hard to justify that we need qemu to be the 
oldstyle server for such interoperability testing (and I can always keep 
an older qemu binary around).

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   include/block/nbd.h |  3 +--
>   blockdev-nbd.c      |  2 +-
>   nbd/server.c        | 53 +++++++++++++--------------------------------
>   qemu-nbd.c          |  2 +-
>   4 files changed, 18 insertions(+), 42 deletions(-)
> 

> +++ b/blockdev-nbd.c
> @@ -36,7 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
>                          gpointer opaque)
>   {
>       qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
> -    nbd_client_new(NULL, cioc,
> +    nbd_client_new(cioc,
>                      nbd_server->tlscreds, NULL,

Could rewrap into fewer lines, but that's cosmetic.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Vladimir Sementsov-Ogievskiy]

03.10.2018 20:32, Eric Blake wrote:
On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
It's unexpected behavior that without -x option qemu-nbd do old-style
negotiation. Let's use "" as a default name instead (as it is already
done if tls is used) and therefore, drop old-style negotiation from
Qemu NBD server.

Oddly enough, I wrote a similar patch in parallel, and am only now just seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you ripped it out altogether).  The client can negotiate either style, so we don't need an option on the client side; rather, this is all about what the server should do by default.

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html

Does anyone have a preference between the two? Here's the last time it was discussed:

https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html

Hm, I think everyone who like new-style should answer that he don't care (except stricter way is a bit better: don't have options and code which we don't need).
But if there is someone who has client which support only old-style negotiation his unhappiness (in case of strict way) will outweigh all our "bits":)

I'm from the first group). But let's chose your patch



Vladimir Sementsov-Ogievskiy (2):
   qemu-nbd: drop old-style negotiation
   nbd/server: drop old-style negotiation

  include/block/nbd.h |  3 +--
  blockdev-nbd.c      |  2 +-
  nbd/server.c        | 53 +++++++++++++--------------------------------
  qemu-nbd.c          | 25 +++++----------------
  4 files changed, 23 insertions(+), 60 deletions(-)





--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Eric Blake]

On 10/3/18 12:59 PM, Vladimir Sementsov-Ogievskiy wrote:
> 03.10.2018 20:32, Eric Blake wrote:
> On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> It's unexpected behavior that without -x option qemu-nbd do old-style
> negotiation. Let's use "" as a default name instead (as it is already
> done if tls is used) and therefore, drop old-style negotiation from
> Qemu NBD server.

Hmm, your email quoting style changed from prior emails that used to 
prepend '>' when quoting, making it harder to tell where the text you 
are quoting ends,...

> 
> Oddly enough, I wrote a similar patch in parallel, and am only now just seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you ripped it out altogether).  The client can negotiate either style, so we don't need an option on the client side; rather, this is all about what the server should do by default.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html
> 
> Does anyone have a preference between the two? Here's the last time it was discussed:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html
> 

and your reply begins.

> Hm, I think everyone who like new-style should answer that he don't care (except stricter way is a bit better: don't have options and code which we don't need).
> But if there is someone who has client which support only old-style negotiation his unhappiness (in case of strict way) will outweigh all our "bits":)
> 
> I'm from the first group). But let's chose your patch

My argument in favor of your patch over mine: nbdkit is a GREAT testbed 
for forcing all sorts of integration testing scenarios, including 
oldstyle servers.  Also, it includes a plugin for translating between 
new and oldstyle at will.  That is, if we ever legitimately encounter a 
client that can only talk oldstyle, but qemu only talks newstyle, we 
just tell the user to connect:

old client => nbdkit -o nbd => newstyle qemu

and then qemu doesn't have to worry about oldstyle because nbdkit does 
instead.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Richard W.M. Jones]

FWIW I don't have anything to add - agree with what's been said already.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Eric Blake]

On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> It's unexpected behavior that without -x option qemu-nbd do old-style
> negotiation. Let's use "" as a default name instead (as it is already
> done if tls is used) and therefore, drop old-style negotiation from
> Qemu NBD server.
> 
> Vladimir Sementsov-Ogievskiy (2):
>    qemu-nbd: drop old-style negotiation
>    nbd/server: drop old-style negotiation
> 
>   include/block/nbd.h |  3 +--
>   blockdev-nbd.c      |  2 +-
>   nbd/server.c        | 53 +++++++++++++--------------------------------
>   qemu-nbd.c          | 25 +++++----------------
>   4 files changed, 23 insertions(+), 60 deletions(-)
> 

Thanks; applying this series to my NBD queue.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Vladimir Sementsov-Ogievskiy]

03.10.2018 21:08, Eric Blake wrote:
> On 10/3/18 12:59 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 03.10.2018 20:32, Eric Blake wrote:
>> On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote:
>> It's unexpected behavior that without -x option qemu-nbd do old-style
>> negotiation. Let's use "" as a default name instead (as it is already
>> done if tls is used) and therefore, drop old-style negotiation from
>> Qemu NBD server.
>
> Hmm, your email quoting style changed from prior emails that used to 
> prepend '>' when quoting, making it harder to tell where the text you 
> are quoting ends,...

I didn't change anything...

>
>>
>> Oddly enough, I wrote a similar patch in parallel, and am only now 
>> just seeing your mail. Yours is a bit stronger than mine (I added 
>> 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you 
>> ripped it out altogether).  The client can negotiate either style, so 
>> we don't need an option on the client side; rather, this is all about 
>> what the server should do by default.
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html
>>
>> Does anyone have a preference between the two? Here's the last time 
>> it was discussed:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html
>>
>
> and your reply begins.
>
>> Hm, I think everyone who like new-style should answer that he don't 
>> care (except stricter way is a bit better: don't have options and 
>> code which we don't need).
>> But if there is someone who has client which support only old-style 
>> negotiation his unhappiness (in case of strict way) will outweigh all 
>> our "bits":)
>>
>> I'm from the first group). But let's chose your patch
>
> My argument in favor of your patch over mine: nbdkit is a GREAT 
> testbed for forcing all sorts of integration testing scenarios, 
> including oldstyle servers.  Also, it includes a plugin for 
> translating between new and oldstyle at will.  That is, if we ever 
> legitimately encounter a client that can only talk oldstyle, but qemu 
> only talks newstyle, we just tell the user to connect:
>
> old client => nbdkit -o nbd => newstyle qemu
>
> and then qemu doesn't have to worry about oldstyle because nbdkit does 
> instead.
>

this is a bit more difficult and more overhead than option in Qemu.. But 
I don't sure we should care about

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation

[Richard W.M. Jones]

On Thu, Oct 04, 2018 at 12:10:17PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> this is a bit more difficult and more overhead than option in Qemu.. But 
> I don't sure we should care about

I think Eric's point is that dropping oldstyle in qemu-nbd isn't
really a problem because nbdkit has no plans to drop oldstyle.  So
there will remain a path for very old clients to keep working (whether
using nbdkit natively, or using nbdkit to proxy to qemu-nbd).  Eric's
example used the more complex proxy case.  By contrast the nbdkit
native case is very simple:

  nbdkit -o file disk.img

(where the ‘-o’ flag switches to oldstyle protocol).

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/