2:56 p.m.
Hi,
qemudDomainBlockStats needs to map between qemu and libvirt device
naming as well. Possible patch attached.
Cheers,
-- Guido
3:06 p.m.
On Thu, Oct 02, 2008 at 09:56:51PM +0200, Guido G?nther wrote:
Hi,
qemudDomainBlockStats needs to map between qemu and libvirt device
naming as well. Possible patch attached.
Cheers,
-- Guido
>From 57b8c8e66abfcc57c2f05d8b8364de6ecc05dcf9 Mon Sep 17 00:00:00
2001
From: Guido Guenther <agx(a)sigxcpu.org>
Date: Thu, 2 Oct 2008 21:12:20 +0200
Subject: [PATCH] support virtio and scsi disks in qemudDomainBlockStats
---
src/qemu_driver.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index a1e7285..2a7c555 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -3416,6 +3416,8 @@ qemudDomainBlockStats (virDomainPtr dom,
* hd[a-] to ide0-hd[0-]
* cdrom to ide1-cd0
* fd[a-] to floppy[0-]
+ * vd[a-] to virtio[0-]
+ * sd[a-] to scsi0-hd[0-]
*/
if (STRPREFIX (path, "hd") && c_islower(path[2]))
snprintf (qemu_dev_name, sizeof (qemu_dev_name),
@@ -3425,6 +3427,12 @@ qemudDomainBlockStats (virDomainPtr dom,
else if (STRPREFIX (path, "fd") && c_islower(path[2]))
snprintf (qemu_dev_name, sizeof (qemu_dev_name),
"floppy%d", path[2] - 'a');
+ else if (STRPREFIX (path, "vd") && c_islower(path[2]))
+ snprintf (qemu_dev_name, sizeof (qemu_dev_name),
+ "virtio%d", path[2] - 'a');
+ else if (STRPREFIX (path, "sd") && c_islower(path[2]))
+ snprintf (qemu_dev_name, sizeof (qemu_dev_name),
+ "scsi0-hd%d", path[2] - 'a');
This isn't correct - it is assuming only a single letter after
the prefix - while true for IDE & Floppy, it doesn't hold true for
SCSI which can have more than 26 devices, so goes to double letters.
We also need to consider Xen style xvdXX devices for Xenner.
I think this needs to use the virDiskNameToIndex() method to
extract the index instead.
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 :|
2:03 p.m.
Guido Günther wrote:
On Thu, Oct 02, 2008 at 09:06:25PM +0100, Daniel P. Berrange wrote:
> I think this needs to use the virDiskNameToIndex() method to
> extract the index instead.
Corrected pach attached. I don't know how devices are called when using
xenner so I left that out.
-- Guido
There is actually a function in qemu_driver.c called
qemudDiskDeviceName that handles a lot of this already,
it's used for generating the connect/eject cdrom names.
It's probably best to reuse that.
Thanks,
Cole
10:23 a.m.
On Tue, Oct 07, 2008 at 03:03:10PM -0400, Cole Robinson wrote:
Guido Günther wrote:
> On Thu, Oct 02, 2008 at 09:06:25PM +0100, Daniel P. Berrange wrote:
>> I think this needs to use the virDiskNameToIndex() method to
>> extract the index instead.
> Corrected pach attached. I don't know how devices are called when using
> xenner so I left that out.
> -- Guido
>
>
There is actually a function in qemu_driver.c called
qemudDiskDeviceName that handles a lot of this already,
it's used for generating the connect/eject cdrom names.
It's probably best to reuse that.
Yes this makes sense. After fixing things up
a little in
virDiskNameToBusDeviceIndex and qemudDiskDeviceName this works. I'll
post this as 3 tiny patches since they might be helpful on their own and
can be applied in case there's still something wrong with the
blockstats.
-- Guido
10:25 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: fix block stats for virtio and scsi
Hi,
0 looks like a valid index as returned by virDiskNameToIndex, so we
should accept it. Also the disk argument can be const.
-- Guido
11:57 a.m.
New subject: [libvirt] Re: [PATCH 1/3] qemu: fix block stats for virtio and scsi
Guido Günther wrote:
Hi,
0 looks like a valid index as returned by virDiskNameToIndex, so we
should accept it. Also the disk argument can be const.
-- Guido
ACK, needed to accommodate recent change to virDiskNameToIndex
at the least.
Thanks,
Cole
12:03 p.m.
New subject: [libvirt] Re: [PATCH 1/3] qemu: fix block stats for virtio and scsi
On Fri, Oct 10, 2008 at 05:25:28PM +0200, Guido G?nther wrote:
Hi,
0 looks like a valid index as returned by virDiskNameToIndex, so we
should accept it. Also the disk argument can be const.
ACK
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 :|
4:18 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: fix block stats for virtio and scsi
On Fri, Oct 10, 2008 at 05:25:28PM +0200, Guido Günther wrote:
Hi,
0 looks like a valid index as returned by virDiskNameToIndex, so we
should accept it. Also the disk argument can be const.
Okidoc, applied,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:27 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: fix block stats for virtio and scsi
Hi,
qemuDiskDeviceName treat's all IDE and SCSI devices as cdroms, make it
also handle disks. Make funcion arguments const.
-- Guido
12:03 p.m.
New subject: [libvirt] Re: [PATCH 2/3] qemu: fix block stats for virtio and scsi
On Fri, Oct 10, 2008 at 05:27:24PM +0200, Guido G?nther wrote:
Hi,
qemuDiskDeviceName treat's all IDE and SCSI devices as cdroms, make it
also handle disks. Make funcion arguments const.
ACK
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 :|
4:24 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: fix block stats for virtio and scsi
On Fri, Oct 10, 2008 at 05:27:24PM +0200, Guido Günther wrote:
Hi,
qemuDiskDeviceName treat's all IDE and SCSI devices as cdroms, make it
also handle disks. Make funcion arguments const.
Applied too to CVS,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:28 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: fix block stats for virtio and scsi
Hi,
use qemudDiskDeviceName to determine the block device name (as suggested
by Cole).
-- Guido
11:57 a.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
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 <agx(a)sigxcpu.org>
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-]
- * cdrom to 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
12:05 p.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
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 :|
8:27 a.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
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
8:29 a.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
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
10:29 a.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
On Tue, Oct 14, 2008 at 09:29:38AM -0400, Cole Robinson wrote:
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.
So can the be applied (together with the other two fixups)?
-- Guido
4:30 a.m.
New subject: [libvirt] Re: [PATCH 3/3] qemu: fix block stats for virtio and scsi
On Thu, Oct 16, 2008 at 05:29:58PM +0200, Guido Günther wrote:
On Tue, Oct 14, 2008 at 09:29:38AM -0400, Cole Robinson wrote:
> 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.
So can the be applied (together with the other two fixups)?
Sure, all 3 commited now,
thanks a lot !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5913
days inactive
5928
days old
19 comments
4 participants
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Guido Günther