[libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi

2008-10-14 Thread Cole Robinson 
Guido Günther wrote:
 On Fri, Oct 10, 2008 at 06:05:37PM +0100, Daniel P. Berrange wrote:
 Sorry to mess up your patch, but I just committed the code to turn
 all linked lists into arrays. So you'll need to tweak this to do
  
   for (i = 0 ; i  vm-def-ndisks ; i++) 
  if (STREQ(vm-def-disks[i]-dst))
  break;
 Fixed version attached. This one also addresses Cole's comment on the
 error message.
  -- Guido
 

ACK.

Thanks,
Cole

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi

2008-10-11 Thread Guido Günther 
On Fri, Oct 10, 2008 at 06:05:37PM +0100, Daniel P. Berrange wrote:
 Sorry to mess up your patch, but I just committed the code to turn
 all linked lists into arrays. So you'll need to tweak this to do
  
   for (i = 0 ; i  vm-def-ndisks ; i++) 
  if (STREQ(vm-def-disks[i]-dst))
  break;
Fixed version attached. This one also addresses Cole's comment on the
error message.
 -- Guido
From 223298503f48db3a4f743ad8d69e8ba165b820df Mon Sep 17 00:00:00 2001
From: Guido Guenther [EMAIL PROTECTED]
Date: Thu, 2 Oct 2008 21:12:20 +0200
Subject: [PATCH] support virtio and scsi disks in qemudDomainBlockStats

---
 src/qemu_driver.c |   54 ++--
 1 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index a49c8ec..5a9d4c5 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2755,11 +2755,13 @@ qemudDomainBlockStats (virDomainPtr dom,
 {
 struct qemud_driver *driver =
 (struct qemud_driver *)dom-conn-privateData;
-char *dummy, *info;
+char *dummy, *info = NULL;
 const char *p, *eol;
-char qemu_dev_name[32];
+const char *qemu_dev_name = NULL;
 size_t len;
+int i, ret = -1;
 const virDomainObjPtr vm = virDomainFindByID(driver-domains, dom-id);
+virDomainDiskDefPtr disk = NULL;
 
 if (!vm) {
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -2772,36 +2774,29 @@ qemudDomainBlockStats (virDomainPtr dom,
 return -1;
 }
 
-/*
- * QEMU internal block device names are different from the device
- * names we use in libvirt, so we need to map between them:
- *
- *   hd[a-]   to  ide0-hd[0-]
- *   cdromto  ide1-cd0
- *   fd[a-]   to  floppy[0-]
- */
-if (STRPREFIX (path, hd)  c_islower(path[2]))
-snprintf (qemu_dev_name, sizeof (qemu_dev_name),
-  ide0-hd%d, path[2] - 'a');
-else if (STREQ (path, cdrom))
-strcpy (qemu_dev_name, ide1-cd0);
-else if (STRPREFIX (path, fd)  c_islower(path[2]))
-snprintf (qemu_dev_name, sizeof (qemu_dev_name),
-  floppy%d, path[2] - 'a');
-else {
+for (i = 0 ; i  vm-def-ndisks ; i++) {
+if (STREQ(path, vm-def-disks[i]-dst)) {
+disk = vm-def-disks[i];
+break;
+}
+}
+
+if (!disk) {
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_ARG,
   _(invalid path: %s), path);
 return -1;
 }
 
+qemu_dev_name = qemudDiskDeviceName(dom, disk);
+if (!qemu_dev_name)
+return -1;
 len = strlen (qemu_dev_name);
 
 if (qemudMonitorCommand (driver, vm, info blockstats, info)  0) {
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
   %s, _('info blockstats' command failed));
-return -1;
+goto out;
 }
-
 DEBUG (info blockstats reply: %s, info);
 
 /* If the command isn't supported then qemu prints the supported
@@ -2810,11 +2805,10 @@ qemudDomainBlockStats (virDomainPtr dom,
  * to detect if qemu supports the command.
  */
 if (STRPREFIX (info, info )) {
-free (info);
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT,
   %s,
   _('info blockstats' not supported by this qemu));
-return -1;
+goto out;
 }
 
 stats-rd_req = -1;
@@ -2865,8 +2859,8 @@ qemudDomainBlockStats (virDomainPtr dom,
 if (!p || p = eol) break;
 p++;
 }
-
-goto done;
+ret = 0;
+goto out;
 }
 
 /* Skip to next line. */
@@ -2876,14 +2870,12 @@ qemudDomainBlockStats (virDomainPtr dom,
 }
 
 /* If we reach here then the device was not found. */
-free (info);
 qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_ARG,
   _(device not found: %s (%s)), path, qemu_dev_name);
-return -1;
-
- done:
-free (info);
-return 0;
+ out:
+VIR_FREE(qemu_dev_name);
+VIR_FREE(info);
+return ret;
 }
 
 static int
-- 
1.5.6.5

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi

2008-10-10 Thread Cole Robinson 
Guido Günther wrote:
 Hi,
 use qemudDiskDeviceName to determine the block device name (as suggested
 by Cole).
  -- Guido
 

 From 6985fee585561b04942036d283632e7cb2bb708f Mon Sep 17 00:00:00 2001
 From: Guido Guenther [EMAIL PROTECTED]
 Date: Thu, 2 Oct 2008 21:12:20 +0200
 Subject: [PATCH] support virtio and scsi disks in qemudDomainBlockStats
 
 ---
  src/qemu_driver.c |   56 
  1 files changed, 26 insertions(+), 30 deletions(-)
 
 diff --git a/src/qemu_driver.c b/src/qemu_driver.c
 index b2ea0d1..5ffaf21 100644
 --- a/src/qemu_driver.c
 +++ b/src/qemu_driver.c
 @@ -3397,11 +3397,13 @@ qemudDomainBlockStats (virDomainPtr dom,
  {
  const struct qemud_driver *driver =
  (struct qemud_driver *)dom-conn-privateData;
 -char *dummy, *info;
 +char *dummy, *info = NULL;
  const char *p, *eol;
 -char qemu_dev_name[32];
 +const char *qemu_dev_name = NULL;
  size_t len;
 +int ret = -1;
  const virDomainObjPtr vm = virDomainFindByID(driver-domains, dom-id);
 +virDomainDiskDefPtr disk;
  
  if (!vm) {
  qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
 @@ -3414,34 +3416,31 @@ qemudDomainBlockStats (virDomainPtr dom,
  return -1;
  }
  
 -/*
 - * QEMU internal block device names are different from the device
 - * names we use in libvirt, so we need to map between them:
 - *
 - *   hd[a-]   to  ide0-hd[0-]
 - *   cdromto  ide1-cd0
 - *   fd[a-]   to  floppy[0-]
 - */
 -if (STRPREFIX (path, hd)  c_islower(path[2]))
 -snprintf (qemu_dev_name, sizeof (qemu_dev_name),
 -  ide0-hd%d, path[2] - 'a');
 -else if (STREQ (path, cdrom))
 -strcpy (qemu_dev_name, ide1-cd0);
 -else if (STRPREFIX (path, fd)  c_islower(path[2]))
 -snprintf (qemu_dev_name, sizeof (qemu_dev_name),
 -  floppy%d, path[2] - 'a');
 -else {
 +disk = vm-def-disks;
 +while (disk) {
 +if (STREQ(disk-dst, path))
 +break;
 +disk = disk-next;
 +}
 +
 +if (!disk) {
  qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_ARG,
_(invalid path: %s), path);
  return -1;
  }
  
 +qemu_dev_name = qemudDiskDeviceName(dom, disk);
 +if (!qemu_dev_name) {
 +qemudReportError (dom-conn, dom, NULL, VIR_ERR_INVALID_ARG,
 +  _(unknown path %s), path);
 +return -1;
 +}

You should probably just return -1 here, since qemudDiskDeviceName
sets its own error message.

The rest looks fine to me.

Thanks,
Cole

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi

2008-10-10 Thread Daniel P. Berrange 
On Fri, Oct 10, 2008 at 05:28:54PM +0200, Guido G?nther wrote:
 Hi,
 use qemudDiskDeviceName to determine the block device name (as suggested
 by Cole).
  -- Guido

 +disk = vm-def-disks;
 +while (disk) {
 +if (STREQ(disk-dst, path))
 +break;
 +disk = disk-next;
 +}

Sorry to mess up your patch, but I just committed the code to turn
all linked lists into arrays. So you'll need to tweak this to do
 
  for (i = 0 ; i  vm-def-ndisks ; i++) 
 if (STREQ(vm-def-disks[i]-dst))
 break;


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list