3:17 p.m.
Hi,
attached is some basic support for host device passthrough. It enables
you to passthrough usb devices in qemu/kvm via:
<devices>
<hostdev type='usb' vendor='0204' product='6025'/>
<hostdev type='usb' bus='001' device='007'/>
</devices>
I didn't implement unplug yet since this needs some modifications to
qemu/kvm to be able to identify the correct device to unplug.
Does this look reasonable?
-- Guido
8:26 a.m.
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
Hi,
attached is some basic support for host device passthrough. It enables
you to passthrough usb devices in qemu/kvm via:
<devices>
<hostdev type='usb' vendor='0204' product='6025'/>
you meant type='pci' there right ?
<hostdev type='usb' bus='001'
device='007'/>
</devices>
Hum, on the format level I think this need a bit more discussion.
the type is better caracterized as a bus informations.
Also I'm not sure if we should keep a flat single element or
(expecting possible further complex descriptions later) go for a
more structured description like
<hostdev type='pci'>
<source vendor='0204' product='6025'/>
</hostdev>
I think an hypervisor could remap the actual port/addresses
of a device so a target may make sense in the future (or not).
I didn't implement unplug yet since this needs some modifications
to
qemu/kvm to be able to identify the correct device to unplug.
Does this look reasonable?
I also think we need to clarify the naming conventions, are numbers
provided decimal, if yes then is an 0x hexadecimal version allowed too.
I also see how a more high level description might be useful,
<hostdev type='usb'>
<source vendor='Sennheiser'/>
</hostdev>
or
<hostdev type='usb'>
<source product='headset'/>
</hostdev>
in that case this means that libvirt has some lookup to do, but in the
end the XML format becomes more portable which is an important property IMHO.
From an implementation perspective, the patch looks very clean to me
aside maybe some Hostdev type vs. Bus naming for the enum, but maybe we need
to give a bit more thoughs one the format first :-)
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:51 a.m.
Hi Daniel,
On Mon, Jul 28, 2008 at 09:26:57AM -0400, Daniel Veillard wrote:
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> Hi,
> attached is some basic support for host device passthrough. It enables
> you to passthrough usb devices in qemu/kvm via:
>
> <devices>
> <hostdev type='usb' vendor='0204' product='6025'/>
you meant type='pci' there right ?
Qemu support host:bus.device syntax
for usb devices (useful for the case
of two identical devices). PCI can work like this too of course.
> <hostdev type='usb' bus='001'
device='007'/>
> </devices>
Hum, on the format level I think this need a bit more discussion.
the type is better caracterized as a bus informations.
Also I'm not sure if we should keep a flat single element or
(expecting possible further complex descriptions later) go for a
more structured description like
<hostdev type='pci'>
<source vendor='0204' product='6025'/>
</hostdev>
I think this looks better. Actually I thought so already after
sending
the patch.
I think an hypervisor could remap the actual port/addresses
of a device so a target may make sense in the future (or not).
Hopefully it does.
It would be nice if we could do that since we'd then
have the necessary data for easy unmapping.
> I didn't implement unplug yet since this needs some
modifications to
> qemu/kvm to be able to identify the correct device to unplug.
> Does this look reasonable?
I also think we need to clarify the naming conventions, are numbers
provided decimal, if yes then is an 0x hexadecimal version allowed too.
Currently
they're all hex as outputet by lsusb.
[..snip..]
From an implementation perspective, the patch looks very clean to
me
aside maybe some Hostdev type vs. Bus naming for the enum, but maybe we need
to give a bit more thoughs one the format first :-)
Sure. I'll see what I can
come up with. Thanks for the comments.
-- Guido
10:02 a.m.
New subject: [libvirt] [PATCH/RFC]: file backed usb massstorage
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
attached is some basic support for host device passthrough. It
enables
you to passthrough usb devices in qemu/kvm via:
On top of the hostdev passthrough
(but it's actually totally
independent) I added usb massstorage backed by a file. Very handy for
installer testing where the preseed data is on the USB stick and the
CD/DVD is the installation medium.
At the moment I'm using a dummy target "usbdisk" so we don't have to
check for target == NULL in that many places. Once qemu handles it we
can fill in bus and device address for unplugging. To add a file as usb
massstorage to the guest you can use:
<disk type='file' device='disk'>
<source file='/foo/bar/usbmass.img'/>
<target bus='usb'/>
</disk>
Does this make sense?
-- Guido
5:53 a.m.
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido G?nther wrote:
Hi,
attached is some basic support for host device passthrough. It enables
you to passthrough usb devices in qemu/kvm via:
<devices>
<hostdev type='usb' vendor='0204' product='6025'/>
<hostdev type='usb' bus='001' device='007'/>
</devices>
This stuff is obviously going to have a correlation with the host
device enumeration support I'd offered a design for a few months
back. As such I'd like to try and keep a consistent XML format
between the two. For reference the original mesage was here:
http://www.redhat.com/archives/libvir-list/2008-April/msg00005.html
There were basically two ways to identify a device. Some devices
are 'physical' mapped straight to a piece of hardware (USB device,
or PCI card) and would have '<bus>' element with hardware details.
eg a USB finger print reader appears as:
<device>
<name>usb_device_483_2016_noserial</name>
<key>/org/freedesktop/Hal/devices/usb_device_483_2016_noserial</key>
<parent>/org/freedesktop/Hal/devices/usb_device_0_0_0000_00_1d_3</parent>
<bus type="usb">
<vendor id="1155">SGS Thomson Microelectronics</vendor>
<product id="8214">Fingerprint Reader</product>
<address bus="003" dev="005"/>
</bus>
</device>
Other devices are 'logical' devices, which don't have hardware info
directly associated with them. The reason for this is that one piece
of hardware may present many logical devices each with varying
capabilities. As an example, a wifi card typically exports at least
2 network device - one control interface, and one for traffic.
eg a wireless network interface for data traffic
<device>
<name>net_00_13_02_b9_f9_d3_0</name>
<key>/org/freedesktop/Hal/devices/net_00_13_02_b9_f9_d3_0</key>
<parent>/org/freedesktop/Hal/devices/pci_8086_4227</parent>
<capability type="net">
<hwaddr>00:13:02:b9:f9:d3</hwaddr>
<name>eth0</name>
<capability type="80211"/>
</capability>
</device>
In this case the unique device identifier is the '<name>' field
but this case varying depending on the capability type.
Different virt solutions have different capabilties for device
passthrough. KVM and Xen both support passthrough of physical
devices, either USB or PCI cards. OpenVZ supports passthrough
of logical devices - in particular network interfaces.
We need to allow for both possibilities in the domain XML for
this type of device.
So, to expand on your proposal, I'd like to see the XML format
have a top level attribute indicating whether we're passing a
logical or physical device, and then the capability name or
bus name respectively. The child elements then need to provide
the appropriate naming.
USB has the further annoyance you identified that one could
conceivably do attachment based on USB bus address, or the
vendor+product pair. The downside of former is that a bus
address changes every time you plug a device in. The downside
of the latter is that it is not neccessarily unique. We have
no choice but to allow both I'm afraid :-(
Finally, with some systems we may have the option of specifying
a target address - eg PCI device ID seen in guest.
So, some examples....
A USB device by vendor+product
<hostdev mode='bus' bus='usb'>
<source>
<product id='1155'/>
<vendor id='8214'/>
</source>
</hostdev>
A USB device by address
<hostdev mode='bus' bus='usb'>
<source>
<address bus='003' dev='005'/>
</source>
</hostdev>
A PCI device by address
<hostdev mode='bus' bus='pci'>
<source>
<address domain="0000" bus="00" slot="16"
function="3"/>
</source>
</hostdev>
A network card by name (ie for OpenVZ)
<hostdev mode='capability'>
<source name='eth0'/>
</hostdev>
A SCSI device by name (eg, SCSI PV passthrough), also specifying
the target adress
<hostdev mode='capability' type='scsi'>
<source name='sg3'/>
<target address='0:0:0:0'/>
</hostdev>
Conceivably we could allow PCI devices by vendor+product, but
I don't see much call for that since PCI device's don't (yet)
appear/disappear on the fly & have a consistent address. More
to the point none of our underlying hypervisors use anything
other than the PCI address for PCI device passthrough.
For USB, if we're doing attachment based on vendor+product,
then libvirt needs to query QEMU to find out the actual
device it chose, so we can fill in the <address> tag. NB I
know QEMU doesn't allow this, but we need it in order todo
unplug reliably, so we'll likely need to do it anyway.
diff --git a/src/domain_conf.h b/src/domain_conf.h
index b438bc8..1aa5c39 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -257,7 +257,35 @@ struct _virDomainGraphicsDef {
} data;
};
+enum virDomainHostdevType {
+ VIR_DOMAIN_HOSTDEV_TYPE_USB,
+ VIR_DOMAIN_HOSTDEV_TYPE_PCI,
+ VIR_DOMAIN_HOSTDEV_TYPE_LAST
+};
+
+typedef struct _virDomainHostdevDef virDomainHostdevDef;
+typedef virDomainHostdevDef *virDomainHostdevDefPtr;
+struct _virDomainHostdevDef {
+ int type;
+ union {
+ struct {
+ int byModel;
+ union {
+ unsigned vendor;
+ unsigned bus;
+ };
+ union {
+ unsigned product;
+ unsigned device;
+ };
+ } usb;
+ struct {
+ /* TBD */
+ } pci;
+ };
+ virDomainHostdevDefPtr next;
+};
Taking into account the various options we need to cope with
I think we'll need something a little larger, along the lines
of
enum virDomainHostdevMode {
VIR_DOMAIN_HSOTDEV_MODE_BUS,
VIR_DOMAIN_HSOTDEV_MODE_CAPABILITY,
};
enum virDomainHostdevBusType
VIR_DOMAIN_HSOTDEV_BUS_TYPE_PCI,
VIR_DOMAIN_HSOTDEV_BUS_TYPE_USB,
};
enum virDomainHostdevCapabilityType {
VIR_DOMAIN_HSOTDEV_CAP_TYPE_NET,
VIR_DOMAIN_HSOTDEV_CAP_TYPE_SCSI,
};
struct _virDomainHostdevDef {
int mode; /* enum virDomainHostdevMode */
union {
struct {
int type; /* enum virDomainHostdevBusType */
union {
struct {
unsigned bus;
unsigned device;
unsigned vendor;
unsigned product;
} usb;
struct {
unsigned domain;
unsigned bus;
unsigned slot;
unsigned function;
} pci;
} data;
} bus;
struct {
int type; /* enum virDomainHostdevCapabilityType */
union {
struct {
char *name;
} net;
struct {
char *name;
} scsi;
};
} cap;
} source;
char *target;
};
/* Flags for the 'type' field in next struct */
enum virDomainDeviceType {
@@ -265,6 +293,7 @@ enum virDomainDeviceType {
VIR_DOMAIN_DEVICE_NET,
VIR_DOMAIN_DEVICE_INPUT,
VIR_DOMAIN_DEVICE_SOUND,
+ VIR_DOMAIN_DEVICE_HOST,
};
typedef struct _virDomainDeviceDef virDomainDeviceDef;
@@ -276,6 +305,7 @@ struct _virDomainDeviceDef {
virDomainNetDefPtr net;
virDomainInputDefPtr input;
virDomainSoundDefPtr sound;
+ virDomainHostdevDefPtr hostdev;
Careful with indentation - check the HACKING file for an
emacs whitespace rule setting.
Modulo, the comments about the XML format, I think your patch is basically
sound & following all our guidelines which is great :-)
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 :|
5:56 a.m.
On Mon, Jul 28, 2008 at 09:26:57AM -0400, Daniel Veillard wrote:
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> Hi,
> attached is some basic support for host device passthrough. It enables
> you to passthrough usb devices in qemu/kvm via:
>
> <devices>
> <hostdev type='usb' vendor='0204' product='6025'/>
you meant type='pci' there right ?
> <hostdev type='usb' bus='001' device='007'/>
> </devices>
Hum, on the format level I think this need a bit more discussion.
the type is better caracterized as a bus informations.
Also I'm not sure if we should keep a flat single element or
(expecting possible further complex descriptions later) go for a
more structured description like
Yes, we need deeper XML structure for the source info
<hostdev type='pci'>
<source vendor='0204' product='6025'/>
</hostdev>
I think an hypervisor could remap the actual port/addresses
of a device so a target may make sense in the future (or not).
Yes, for SCSI at least we need target info, and if we can query
the mapped target address from the underlying hypervisor we
should display it.
> I didn't implement unplug yet since this needs some modifications to
> qemu/kvm to be able to identify the correct device to unplug.
> Does this look reasonable?
I also think we need to clarify the naming conventions, are numbers
provided decimal, if yes then is an 0x hexadecimal version allowed too.
I also see how a more high level description might be useful,
<hostdev type='usb'>
<source vendor='Sennheiser'/>
</hostdev>
or
<hostdev type='usb'>
<source product='headset'/>
</hostdev>
We should stick to ID numbers in the domain XML - allowing decimal
or hex is nice.
For descriptive names 'Sennheiser' or 'headset', we should only
provide that info in the device enumeration APIs
http://www.redhat.com/archives/libvir-list/2008-April/msg00005.html
Management tools / admins can use this to map to the ID number
from a human readable name if relevant.
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 :|
6:04 a.m.
On Tue, Jul 29, 2008 at 11:53:46AM +0100, Daniel P. Berrange wrote:
This stuff is obviously going to have a correlation with the host
device enumeration support I'd offered a design for a few months
back. As such I'd like to try and keep a consistent XML format
between the two. For reference the original mesage was here:
http://www.redhat.com/archives/libvir-list/2008-April/msg00005.html
There were basically two ways to identify a device. Some devices
are 'physical' mapped straight to a piece of hardware (USB device,
or PCI card) and would have '<bus>' element with hardware details.
Oh, one thing I meant to say. Contrary to my message in the thread
above, and my previous reply, we should actually use 'subsys' or
'subsystem' instead of 'bus', to follow the HAL naming.
eg a USB finger print reader appears as:
<device>
<name>usb_device_483_2016_noserial</name>
<key>/org/freedesktop/Hal/devices/usb_device_483_2016_noserial</key>
<parent>/org/freedesktop/Hal/devices/usb_device_0_0_0000_00_1d_3</parent>
<bus type="usb">
eg
<subsystem type='usb'>
<vendor id="1155">SGS Thomson
Microelectronics</vendor>
<product id="8214">Fingerprint Reader</product>
<address bus="003" dev="005"/>
</bus>
</device>
Other devices are 'logical' devices, which don't have hardware info
directly associated with them. The reason for this is that one piece
of hardware may present many logical devices each with varying
capabilities. As an example, a wifi card typically exports at least
2 network device - one control interface, and one for traffic.
eg a wireless network interface for data traffic
<device>
<name>net_00_13_02_b9_f9_d3_0</name>
<key>/org/freedesktop/Hal/devices/net_00_13_02_b9_f9_d3_0</key>
<parent>/org/freedesktop/Hal/devices/pci_8086_4227</parent>
<capability type="net">
<hwaddr>00:13:02:b9:f9:d3</hwaddr>
<name>eth0</name>
<capability type="80211"/>
</capability>
</device>
In this case the unique device identifier is the '<name>' field
but this case varying depending on the capability type.
Different virt solutions have different capabilties for device
passthrough. KVM and Xen both support passthrough of physical
devices, either USB or PCI cards. OpenVZ supports passthrough
of logical devices - in particular network interfaces.
We need to allow for both possibilities in the domain XML for
this type of device.
So, to expand on your proposal, I'd like to see the XML format
have a top level attribute indicating whether we're passing a
logical or physical device, and then the capability name or
bus name respectively. The child elements then need to provide
the appropriate naming.
USB has the further annoyance you identified that one could
conceivably do attachment based on USB bus address, or the
vendor+product pair. The downside of former is that a bus
address changes every time you plug a device in. The downside
of the latter is that it is not neccessarily unique. We have
no choice but to allow both I'm afraid :-(
Finally, with some systems we may have the option of specifying
a target address - eg PCI device ID seen in guest.
So, some examples....
A USB device by vendor+product
<hostdev mode='bus' bus='usb'>
<hostdev mode='subsystem' type='usb'>
<source>
<product id='1155'/>
<vendor id='8214'/>
</source>
</hostdev>
A USB device by address
<hostdev mode='bus' bus='usb'>
<hostdev mode='subsystem' type='usb'>
<source>
<address bus='003' dev='005'/>
</source>
</hostdev>
A PCI device by address
<hostdev mode='bus' bus='pci'>
<hostdev mode='subsystem' type='pci'>
<source>
<address domain="0000" bus="00" slot="16"
function="3"/>
</source>
</hostdev>
A network card by name (ie for OpenVZ)
<hostdev mode='capability'>
<source name='eth0'/>
</hostdev>
A SCSI device by name (eg, SCSI PV passthrough), also specifying
the target adress
<hostdev mode='capability' type='scsi'>
<source name='sg3'/>
<target address='0:0:0:0'/>
</hostdev>
Taking into account the various options we need to cope with
I think we'll need something a little larger, along the lines
of
enum virDomainHostdevMode {
VIR_DOMAIN_HSOTDEV_MODE_BUS,
VIR_DOMAIN_HSOTDEV_MODE_CAPABILITY,
};
enum virDomainHostdevBusType
VIR_DOMAIN_HSOTDEV_BUS_TYPE_PCI,
VIR_DOMAIN_HSOTDEV_BUS_TYPE_USB,
};
s/BUS/SUBSYS/
enum virDomainHostdevCapabilityType {
VIR_DOMAIN_HSOTDEV_CAP_TYPE_NET,
VIR_DOMAIN_HSOTDEV_CAP_TYPE_SCSI,
};
struct _virDomainHostdevDef {
int mode; /* enum virDomainHostdevMode */
union {
struct {
int type; /* enum virDomainHostdevBusType */
union {
struct {
unsigned bus;
unsigned device;
unsigned vendor;
unsigned product;
} usb;
struct {
unsigned domain;
unsigned bus;
unsigned slot;
unsigned function;
} pci;
} data;
} bus;
s/bus/subsys/
struct {
int type; /* enum virDomainHostdevCapabilityType */
union {
struct {
char *name;
} net;
struct {
char *name;
} scsi;
};
} cap;
} source;
char *target;
};
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 :|
7:17 a.m.
On Tue, Jul 29, 2008 at 11:56:18AM +0100, Daniel P. Berrange wrote:
On Mon, Jul 28, 2008 at 09:26:57AM -0400, Daniel Veillard wrote:
> I also think we need to clarify the naming conventions, are numbers
> provided decimal, if yes then is an 0x hexadecimal version allowed too.
> I also see how a more high level description might be useful,
>
> <hostdev type='usb'>
> <source vendor='Sennheiser'/>
> </hostdev>
>
> or
>
> <hostdev type='usb'>
> <source product='headset'/>
> </hostdev>
We should stick to ID numbers in the domain XML - allowing decimal
or hex is nice.
For descriptive names 'Sennheiser' or 'headset', we should only
provide that info in the device enumeration APIs
Well, I was thinking of this as a way to keep the XML file static
over a range of potential hardware, potentially allowing some kind
of migration in spite of a dependancy to a local hardware device.
But that's probably not very realistic, and if turn out possible sometimes
then that can be added later.
http://www.redhat.com/archives/libvir-list/2008-April/msg00005.html
Management tools / admins can use this to map to the ID number
from a human readable name if relevant.
Okay, that solves the problem at creation time, i remember this being
discussed no problem.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
3:47 a.m.
New subject: [libvirt] [PATCH/RFC]: file backed usb massstorage
On Mon, Jul 28, 2008 at 11:02:45AM -0400, Guido G?nther wrote:
On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> attached is some basic support for host device passthrough. It enables
> you to passthrough usb devices in qemu/kvm via:
On top of the hostdev passthrough (but it's actually totally
independent) I added usb massstorage backed by a file. Very handy for
installer testing where the preseed data is on the USB stick and the
CD/DVD is the installation medium.
At the moment I'm using a dummy target "usbdisk" so we don't have to
check for target == NULL in that many places. Once qemu handles it we
can fill in bus and device address for unplugging. To add a file as usb
massstorage to the guest you can use:
<disk type='file' device='disk'>
<source file='/foo/bar/usbmass.img'/>
<target bus='usb'/>
</disk>
The way we do target for other disk types doesn't work so well - others
we typically have a device name, eg hda, sdc, xvdf, etc. For VirtIO we
let people pass in a dummy device name too 'vda', 'vdb', etc. Only some
of the disk buses & guest OS honour these names though - in cases where
they're not honoured, they at most provide a unique key, and allow for
ordering of disks in our XML. USB will be much like SCSI / VirtIO where
the device name in the XML is merely used for ordering, so we should just
allow sda, sdb, etc as the target device name. This will avoid having
to special case the code in the here too much.
---
src/domain_conf.c | 40 ++++++++++++++++++++++++++++------------
src/domain_conf.h | 1 +
src/qemu_conf.c | 23 +++++++++++++++++++++--
src/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 91 insertions(+), 14 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index d36caeb..74ceecc 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -84,7 +84,8 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST,
"fdc",
"scsi",
"virtio",
- "xen")
+ "xen",
+ "usb")
This is fine.
VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
"user",
@@ -540,6 +541,14 @@ virDomainDiskDefParseXML(virConnectPtr conn,
def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
}
+ if (bus) {
+ if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk bus type '%s'"),
bus);
+ goto error;
+ }
+ }
+
No need to move this - see further down.
/* Only CDROM and Floppy devices are allowed missing source
path
* to indicate no media present */
if (source == NULL &&
@@ -550,10 +559,16 @@ virDomainDiskDefParseXML(virConnectPtr conn,
goto error;
}
+ /* only USB devices are allowed missing target path since the hypervisor
+ * can assign bus and device number */
if (target == NULL) {
- virDomainReportError(conn, VIR_ERR_NO_TARGET,
- source ? "%s" : NULL, source);
- goto error;
+ if (def->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ target = strdup("usbdisk");
+ } else {
+ virDomainReportError(conn, VIR_ERR_NO_TARGET,
+ source ? "%s" : NULL, source);
+ goto error;
+ }
No need - we'll just define that 'sdXXXX' is the prefix to use.
}
if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
@@ -571,19 +586,14 @@ virDomainDiskDefParseXML(virConnectPtr conn,
!STRPREFIX((const char *)target, "hd") &&
!STRPREFIX((const char *)target, "sd") &&
!STRPREFIX((const char *)target, "vd") &&
- !STRPREFIX((const char *)target, "xvd")) {
+ !STRPREFIX((const char *)target, "xvd") &&
+ !STREQ((const char*)target, "usbdisk")) {
Again, this is not needed.
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Invalid harddisk device name: %s"), target);
goto error;
}
- if (bus) {
- if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) {
- virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("unknown disk bus type '%s'"),
bus);
- goto error;
- }
- } else {
+ if (!bus) {
if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
def->bus = VIR_DOMAIN_DISK_BUS_FDC;
} else {
@@ -612,6 +622,12 @@ virDomainDiskDefParseXML(virConnectPtr conn,
_("Invalid bus type '%s' for disk"),
bus);
goto error;
}
+ if (def->device != VIR_DOMAIN_DISK_DEVICE_DISK &&
+ def->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Invalid bus type '%s' for usb disk"),
bus);
+ goto error;
+ }
It is possible to have USB floppys and CDROMs. So this check should be
made in the underlying drivers at time of use, rather than in the
parser. eg, move this into qemudBuildCommandLine() method, or the
device hotplug method for QEMU. Similarly for other hypervisor backends
as needed.
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 7678ac5..868b3dc 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -55,7 +55,8 @@ VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
"floppy",
"scsi",
"virtio",
- "xen")
+ "xen",
+ "usb")
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -772,6 +773,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto no_memory; \
} while (0)
+#define ADD_USBDISK(thisarg) \
+ do { \
+ ADD_ARG_LIT("-usbdevice"); \
+ if ((asprintf(&qargv[qargc++], "disk:%s", thisarg)) == -1) \
+ goto no_memory; \
+ } while (0)
+
snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024);
snprintf(vcpus, sizeof(vcpus), "%lu", vm->def->vcpus);
@@ -883,6 +891,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
int idx = virDiskNameToIndex(disk->dst);
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ ADD_USBDISK(disk->src);
+ disk = disk->next;
+ continue;
+ }
+
The indentation is messed up here.
Annoying that QEMU's -drive arg doesn't support USB :-(
if (idx < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("unsupported disk type '%s'"),
disk->dst);
@@ -924,6 +938,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
char dev[NAME_MAX];
char file[PATH_MAX];
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
+ ADD_USBDISK(disk->src);
+ disk = disk->next;
+ continue;
+ }
More indentation issues.
+
if (STREQ(disk->dst, "hdc") &&
disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
if (disk->src) {
@@ -947,7 +967,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
ADD_ARG_LIT(dev);
ADD_ARG_LIT(file);
-
disk = disk->next;
}
}
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 3381d10..73b6da4 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2979,6 +2979,44 @@ static int qemudDomainAttachCdromDevice(virDomainPtr dom,
return 0;
}
+static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr
dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ int ret;
+ char *cmd, *reply;
+
+ ret = asprintf(&cmd, "usb_add disk:%s", dev->data.disk->src);
+
+ if (ret == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("out of memory"));
+ return ret;
+ }
Minor indentation issue there.
+
+ if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("cannot attach usb device"));
+ VIR_FREE(cmd);
+ return -1;
+ }
+
+ DEBUG ("attach_usb reply: %s", reply);
+ /* If the command failed qemu prints:
+ * Could not add ... */
+ if (strstr(reply, "Could not add ")) {
+ qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s",
+ _("adding usb device failed"));
+ VIR_FREE(reply);
+ VIR_FREE(cmd);
+ return -1;
+ }
+ VIR_FREE(reply);
+ VIR_FREE(cmd);
+ return 0;
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 :|
3:17 a.m.
New subject: [libvirt] [PATCH/RFC]: file backed usb massstorage
On Wed, Jul 30, 2008 at 09:47:00AM +0100, Daniel P. Berrange wrote:
On Mon, Jul 28, 2008 at 11:02:45AM -0400, Guido G?nther wrote:
> On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> > attached is some basic support for host device passthrough. It enables
> > you to passthrough usb devices in qemu/kvm via:
> On top of the hostdev passthrough (but it's actually totally
> independent) I added usb massstorage backed by a file. Very handy for
> installer testing where the preseed data is on the USB stick and the
> CD/DVD is the installation medium.
> At the moment I'm using a dummy target "usbdisk" so we don't have
to
> check for target == NULL in that many places. Once qemu handles it we
> can fill in bus and device address for unplugging. To add a file as usb
> massstorage to the guest you can use:
>
> <disk type='file' device='disk'>
> <source file='/foo/bar/usbmass.img'/>
> <target bus='usb'/>
> </disk>
The way we do target for other disk types doesn't work so well - others
we typically have a device name, eg hda, sdc, xvdf, etc. For VirtIO we
let people pass in a dummy device name too 'vda', 'vdb', etc. Only some
of the disk buses & guest OS honour these names though - in cases where
they're not honoured, they at most provide a unique key, and allow for
ordering of disks in our XML. USB will be much like SCSI / VirtIO where
the device name in the XML is merely used for ordering, so we should just
allow sda, sdb, etc as the target device name. This will avoid having
to special case the code in the here too much.
Acutally I intended to use the
target like target="bus:device" once qemu
(and others) support this. The would also help on detach. Would you
prefer:
<target bus='usb' target='sda' bus='0001' id='1234'>
then?
-- Guido
3:39 a.m.
New subject: [libvirt] [PATCH/RFC]: file backed usb massstorage
On Thu, Jul 31, 2008 at 04:17:39AM -0400, Guido G?nther wrote:
On Wed, Jul 30, 2008 at 09:47:00AM +0100, Daniel P. Berrange wrote:
> On Mon, Jul 28, 2008 at 11:02:45AM -0400, Guido G?nther wrote:
> > On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> > > attached is some basic support for host device passthrough. It enables
> > > you to passthrough usb devices in qemu/kvm via:
> > On top of the hostdev passthrough (but it's actually totally
> > independent) I added usb massstorage backed by a file. Very handy for
> > installer testing where the preseed data is on the USB stick and the
> > CD/DVD is the installation medium.
> > At the moment I'm using a dummy target "usbdisk" so we don't
have to
> > check for target == NULL in that many places. Once qemu handles it we
> > can fill in bus and device address for unplugging. To add a file as usb
> > massstorage to the guest you can use:
> >
> > <disk type='file' device='disk'>
> > <source file='/foo/bar/usbmass.img'/>
> > <target bus='usb'/>
> > </disk>
>
> The way we do target for other disk types doesn't work so well - others
> we typically have a device name, eg hda, sdc, xvdf, etc. For VirtIO we
> let people pass in a dummy device name too 'vda', 'vdb', etc. Only
some
> of the disk buses & guest OS honour these names though - in cases where
> they're not honoured, they at most provide a unique key, and allow for
> ordering of disks in our XML. USB will be much like SCSI / VirtIO where
> the device name in the XML is merely used for ordering, so we should just
> allow sda, sdb, etc as the target device name. This will avoid having
> to special case the code in the here too much.
Acutally I intended to use the target like target="bus:device" once qemu
(and others) support this. The would also help on detach. Would you
prefer:
<target bus='usb' target='sda' bus='0001'
id='1234'>
Taking into account the other thread discussing USB/PCI device passthrough
to the guest for host device, I think it'd be worth having an explicit
'address' element for the USB details using a matching syntax, eg
<target dev='sda' bus='usb'>
<address bus='0001' dev='1234'/>
</target>
Regards,
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 :|
6:41 p.m.
New subject: [libvirt] [PATCH/RFC]: hostdev passthrough support take #2
Hi,
attached is a second version. Changes are:
* s/bus/subsystem/
* support hexadecimal and decimal attributes
* introduce device and source elements.
I decided to not drop vendor and product id into their own elements
since the structure would then become very nested for no good reason.
Some examples:
<hostdev mode="subsys" type='usb'>
<source>
<device vendor="0x0204" product="0x6025"/>
</source>
</hostdev>
<hostdev mode="subsys" type='usb'>
<source>
<address bus="001" device="003"/>
</source>
</hostdev>
Support for <target> will be coming once I got around to adjust
qemu/kvm.
Cheers,
-- Guido
6:50 p.m.
New subject: [libvirt] [PATCH/RFC]: file backed usb massstorage #2
Hi,
attached is a second version of the usb massstorage patch:
Changes are:
* use sd* dummy dev names instead of usbdisk
* move the check for the "subtype" (hard disk/cdrom/...) into the driver
An example is:
<disk type='file' device='disk'>
<target dev="sda" bus='usb'/>
<source file='/foo/bar/usbmass.img'/>
</disk>
Patch is on top of the hostdev passthrough patch.
Cheers,
-- Guido
4:34 a.m.
New subject: [libvirt] [PATCH/RFC]: hostdev passthrough support take #2
On Sun, Aug 03, 2008 at 01:41:28AM +0200, Guido G?nther wrote:
Hi,
attached is a second version. Changes are:
* s/bus/subsystem/
* support hexadecimal and decimal attributes
* introduce device and source elements.
I decided to not drop vendor and product id into their own elements
since the structure would then become very nested for no good reason.
The reason is that I want it to match the host device enumeration
XML format. This will be using explicit <product> and <vendor>
tags, with an 'id' attribute, because there will be text content
in the body giving the human readable name.
Some examples:
<hostdev mode="subsys" type='usb'>
<source>
<device vendor="0x0204" product="0x6025"/>
So this needs to be changed to
<vendor id="0x0204"/>
<product id="0x6025"/>
</source>
</hostdev>
<hostdev mode="subsys" type='usb'>
<source>
<address bus="001" device="003"/>
</source>
</hostdev>
This one is fine.
Support for <target> will be coming once I got around to adjust
qemu/kvm.
Ok, that's not critical, but the other things we need before we
can commit this are
- Code to format XML for output, so that the devices are included when
dumping the XML description of a guest.
- Update to the RNG schema in docs/libvirt.rng
- Example XML data files in tests/qemuxml2argvdata/, along with the
corresponding CLI args for QEMU. THis is then hooked into the
qemuxml2argvtest.c and qemuxml2xmltest.c files
The latter test suite requires point the XML formatting & RNG updates.
The currrent code you have for parsing XML & attaching device/creating
CLI flags is basically correct with only very minor bugs
+static int
+virDomainHostdevSubsysUsbDefParseXML(virConnectPtr conn,
+ const xmlNodePtr node,
+ virDomainHostdevDefPtr def) {
Minor indentation bug.
+
+ int ret = -1;
+ xmlNodePtr cur;
+
+ def->source.subsys.usb.vendor = 0;
+ def->source.subsys.usb.product = 0;
+ def->source.subsys.usb.bus = 0;
+ def->source.subsys.usb.device = 0;
This isn't needed, since the VIR_ALLOC contract says that the memory
range has been memset() to all zero.
+static int qemudDomainAttachHostDevice(virDomainPtr dom,
virDomainDeviceDefPtr dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ int ret;
+ char *cmd, *reply;
+
+ if (dev->data.hostdev->source.subsys.usb.vendor) {
+ ret = asprintf(&cmd, "usb_add host:%.4x:%.4x",
+ dev->data.hostdev->source.subsys.usb.vendor,
+ dev->data.hostdev->source.subsys.usb.product);
+ } else {
+ ret = asprintf(&cmd, "usb_add host:%.3d.%.3d",
+ dev->data.hostdev->source.subsys.usb.bus,
+ dev->data.hostdev->source.subsys.usb.device);
+ }
+ if (ret == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("out of memory"));
+ return -1;
This should be reporting OOM, and not pass the 'dom' parameter, and
no need for a string message, eg
qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
Regards,
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:42 a.m.
New subject: [libvirt] Re: [PATCH/RFC]: file backed usb massstorage #2
On Sun, Aug 03, 2008 at 01:50:44AM +0200, Guido G?nther wrote:
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 73039c5..331ff9d 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -883,6 +891,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
int idx = virDiskNameToIndex(disk->dst);
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+ disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ ADD_USBDISK(disk->src);
+ disk = disk->next;
+ continue;
+ }
Should do a 'qemudReportError' if the device type != DISK, so if someone
mistakenly adds a floppy/cdrom it isn't silently ignored.
if (idx < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("unsupported disk type '%s'"),
disk->dst);
@@ -922,6 +937,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
char dev[NAME_MAX];
char file[PATH_MAX];
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+ disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ ADD_USBDISK(disk->src);
+ disk = disk->next;
+ continue;
+ }
Same here.
+
if (STREQ(disk->dst, "hdc") &&
disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
if (disk->src) {
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 1aff53e..018046e 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2978,6 +2978,44 @@ static int qemudDomainAttachCdromDevice(virDomainPtr dom,
return 0;
}
+static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr
dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ int ret;
+ char *cmd, *reply;
+
+ ret = asprintf(&cmd, "usb_add disk:%s", dev->data.disk->src);
+
+ if (ret == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("out of memory"));
Should just be
qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
As with the other patch we need to have an update to the RNG schema
and at least one example test data file in tests/qemuxml2argvdata/
with XML and corresponding ARGV, plumbed into qemuxml2xmltest.c and
qemuxml2argvtest.c
With those changes I'd be happy to see this committed.
Regards,
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 :|
11 a.m.
New subject: [libvirt] [PATCH/RFC]: hostdev passthrough support take #2
On Mon, Aug 04, 2008 at 10:34:31AM +0100, Daniel P. Berrange wrote:
On Sun, Aug 03, 2008 at 01:41:28AM +0200, Guido G?nther wrote:
> Hi,
> attached is a second version. Changes are:
>
> * s/bus/subsystem/
> * support hexadecimal and decimal attributes
> * introduce device and source elements.
>
> I decided to not drop vendor and product id into their own elements
> since the structure would then become very nested for no good reason.
The reason is that I want it to match the host device enumeration
XML format. This will be using explicit <product> and <vendor>
tags, with an 'id' attribute, because there will be text content
in the body giving the human readable name.
Ahh...o.k.
> Some examples:
>
> <hostdev mode="subsys" type='usb'>
> <source>
> <device vendor="0x0204" product="0x6025"/>
So this needs to be changed to
<vendor id="0x0204"/>
<product id="0x6025"/>
Sure.
Ok, that's not critical, but the other things we need before we
can commit this are
- Code to format XML for output, so that the devices are included when
dumping the XML description of a guest.
- Update to the RNG schema in docs/libvirt.rng
- Example XML data files in tests/qemuxml2argvdata/, along with the
corresponding CLI args for QEMU. THis is then hooked into the
qemuxml2argvtest.c and qemuxml2xmltest.c files
Sure. I delayed those since
they'd have to be adjusted when the XML
changes, I'll add them now and do the other minor touchups you
suggested...might be a couple of days though.
-- Guido
11:25 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #3
Hi,
attached is version three of the hostdev passthrough patch. It adds:
* code to format the XML for output
* RelaxNG schema update
* testcases
Cheers,
-- Guido
12:16 p.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
Hi,
attached is version three of the file backed usb massstorage patch.
* handle type != DISK case
* on OOM use VIR_ERR_NO_MEMORY instead of VIR_ERR_OPERAION_FAILED
* RelaxNG schema update
* testcase
The patches are ment to be applied on top of the hostdev patches.
Cheers,
-- Guido
5:46 a.m.
New subject: [libvirt] Re: [PATCH]: hostdev passthrough support take #3
On Thu, Aug 07, 2008 at 06:25:56PM +0200, Guido G?nther wrote:
Hi,
attached is version three of the hostdev passthrough patch. It adds:
* code to format the XML for output
* RelaxNG schema update
* testcases
ACK, this looks ready to commit now - great work getting this done.
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 :|
5:47 a.m.
New subject: [libvirt] Re: [PATCH]: file backed usb massstorage #3
On Thu, Aug 07, 2008 at 07:16:30PM +0200, Guido G?nther wrote:
Hi,
attached is version three of the file backed usb massstorage patch.
* handle type != DISK case
* on OOM use VIR_ERR_NO_MEMORY instead of VIR_ERR_OPERAION_FAILED
* RelaxNG schema update
* testcase
The patches are ment to be applied on top of the hostdev patches.
ACK, also looks good to commit
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 :|
7:07 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
Guido Günther <agx(a)sigxcpu.org> wrote:
attached is version three of the file backed usb massstorage patch.
* handle type != DISK case
* on OOM use VIR_ERR_NO_MEMORY instead of VIR_ERR_OPERAION_FAILED
* RelaxNG schema update
* testcase
...
ACK, once you fix the following:
>From 123763836d1fe71e6180c36a75690ceec0174f8c Mon Sep 17 00:00:00
2001
From: Guido Guenther <agx(a)sigxcpu.org>
Date: Sun, 3 Aug 2008 01:07:00 +0200
Subject: [PATCH] usbmass: use files as USB disks
...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
...
+#define ADD_USBDISK(thisarg)
\
+ do { \
+ ADD_ARG_LIT("-usbdevice"); \
+ ADD_ARG_SPACE; \
+ if ((asprintf(&qargv[qargc++], "disk:%s", thisarg)) == -1) \
+ goto no_memory; \
Set qargv[qargc-1] = NULL before the "goto", since upon asprintf
failure it's undefined -- and the subsequent attempt to free it
may well evoke a segfault.
+ } while (0)
...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ef4e158..118d423 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2978,6 +2978,43 @@ static int qemudDomainAttachCdromDevice(virDomainPtr dom,
return 0;
}
+static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr
dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
This should handle the case in which vm == NULL.
Otherwise, qemudMonitorCommand will dereference NULL.
...
>From a714b14a13f97b824c288fc40165ef9fe5fbaae0 Mon Sep 17 00:00:00
2001
From: Guido Guenther <agx(a)sigxcpu.org>
Date: Thu, 7 Aug 2008 14:39:35 +0200
Subject: [PATCH] usbmass: testcase for file as src for usb disk
...
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
new file mode 100644
index 0000000..60a8317
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
@@ -0,0 +1 @@
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img -net none -serial none -parallel
none -usb
\ No newline at end of file
Please add a newline and split the long line:
/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
-boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
-net none -serial none -parallel none -usb
7:30 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #3
Guido Günther <agx(a)sigxcpu.org> wrote:
attached is version three of the hostdev passthrough patch. It adds:
* code to format the XML for output
* RelaxNG schema update
* testcases
ACK, modulo the details below.
>From cfcfc85accdcc7be7a5fbfd2c8dde435646d5ab2 Mon Sep 17 00:00:00
2001
From: Guido Guenther <agx(a)sigxcpu.org>
Date: Fri, 25 Jul 2008 15:18:16 -0400
Subject: [PATCH] hostdev: pass host devices to the guest
...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 7fe3903..ef4e158 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2951,12 +2951,83 @@ static int qemudDomainChangeCDROM(virDomainPtr dom,
return 0;
}
+static int qemudDomainAttachCdromDevice(virDomainPtr dom,
+ virDomainDeviceDefPtr dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ virDomainDiskDefPtr disk;
+
+ disk = vm->def->disks;
Check for vm == NULL before dereferencing it.
...
+static int qemudDomainAttachHostDevice(virDomainPtr dom,
virDomainDeviceDefPtr dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
...
+ if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
Likewise.
...
>From 6fd13e2faa49eadf2db4a7e4a3a28192b4c61622 Mon Sep 17 00:00:00
2001
From: Guido Guenther <agx(a)sigxcpu.org>
Date: Thu, 7 Aug 2008 14:25:50 +0200
Subject: [PATCH] hostdev: add testcases
...
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args
new file mode 100644
index 0000000..0b89999
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.args
@@ -0,0 +1 @@
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -usbdevice host:014.006
\ No newline at end of file
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
new file mode 100644
index 0000000..b993ae5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
@@ -0,0 +1 @@
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -usbdevice
host:0204:6025
\ No newline at end of file
Please split the long lines and add a newline at EOF.
7:35 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
On Fri, Aug 08, 2008 at 02:07:49PM +0200, Jim Meyering wrote:
Guido Günther <agx(a)sigxcpu.org> wrote:
>>From a714b14a13f97b824c288fc40165ef9fe5fbaae0 Mon Sep 17 00:00:00 2001
> From: Guido Guenther <agx(a)sigxcpu.org>
> Date: Thu, 7 Aug 2008 14:39:35 +0200
> Subject: [PATCH] usbmass: testcase for file as src for usb disk
...
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
> new file mode 100644
> index 0000000..60a8317
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args
> @@ -0,0 +1 @@
> +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img -net none -serial none -parallel
none -usb
> \ No newline at end of file
Please add a newline and split the long line:
/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
-boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
-net none -serial none -parallel none -usb
No, this would break the test case, since its being compared against
generated data in the format shown by the patch.
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 :|
7:51 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Aug 08, 2008 at 02:07:49PM +0200, Jim Meyering wrote:
> Guido Günther <agx(a)sigxcpu.org> wrote:
> >>From a714b14a13f97b824c288fc40165ef9fe5fbaae0 Mon Sep 17 00:00:00 2001
> > From: Guido Guenther <agx(a)sigxcpu.org>
...
> Please add a newline and split the long line:
>
> /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
> -boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
> -net none -serial none -parallel none -usb
No, this would break the test case, since its being compared against
generated data in the format shown by the patch.
Ahh... it looked like something to be executed (and maintained),
rather than merely expected output. So no need to split long lines.
However, including a trailing newline might be worthwhile
(and adjust the code to emit the matching newline), since some editors
have a hard time modifying or creating a file with no newline at EOF.
8:12 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
Jim Meyering wrote:
> On Fri, Aug 08, 2008 at 02:07:49PM +0200, Jim Meyering wrote:
>> Guido Günther <agx(a)sigxcpu.org> wrote:
>>> >From a714b14a13f97b824c288fc40165ef9fe5fbaae0 Mon Sep 17 00:00:00 2001
>>> From: Guido Guenther <agx(a)sigxcpu.org>
...
>> Please add a newline and split the long line:
>>
>> /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
>> -boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
>> -net none -serial none -parallel none -usb
> No, this would break the test case, since its being compared against
> generated data in the format shown by the patch.
Ahh... it looked like something to be executed (and maintained),
rather than merely expected output. So no need to split long lines.
However, including a trailing newline might be worthwhile
(and adjust the code to emit the matching newline), since some editors
have a hard time modifying or creating a file with no newline at EOF.
Heh. Unfortunately, that's not an option either. The tests just do a simple
strcmp(), so if there is an extra trailing newline in the args file, the test
will "fail". I ran into this before.
Chris Lalancette
8:26 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #3
Chris Lalancette <clalance(a)redhat.com> wrote:
Jim Meyering wrote:
>> On Fri, Aug 08, 2008 at 02:07:49PM +0200, Jim Meyering wrote:
>>> Guido Günther <agx(a)sigxcpu.org> wrote:
>>>> >From a714b14a13f97b824c288fc40165ef9fe5fbaae0 Mon Sep 17 00:00:00
2001
>>>> From: Guido Guenther <agx(a)sigxcpu.org>
> ...
>>> Please add a newline and split the long line:
>>>
>>> /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
>>> -boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
>>> -net none -serial none -parallel none -usb
>> No, this would break the test case, since its being compared against
>> generated data in the format shown by the patch.
>
> Ahh... it looked like something to be executed (and maintained),
> rather than merely expected output. So no need to split long lines.
>
> However, including a trailing newline might be worthwhile
> (and adjust the code to emit the matching newline), since some editors
> have a hard time modifying or creating a file with no newline at EOF.
Heh. Unfortunately, that's not an option either. The tests just do a simple
strcmp(), so if there is an extra trailing newline in the args file, the test
will "fail". I ran into this before.
Maybe you're talking about something else?
I proposed to add a trailing newline in the expected output
as well as in the code to generate the "actual" output.
This is the sort of change I was thinking of:
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d136a13..25465ae 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -53,7 +53,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *cmd,
int extra
goto fail;
tmp = argv;
- len = 0;
+ len = 1; /* for trailing newline */
while (*tmp) {
len += strlen(*tmp) + 1;
tmp++;
@@ -68,6 +68,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *cmd,
int extra
strcat(actualargv, *tmp);
tmp++;
}
+ strcat(actualargv, "\n");
if (STRNEQ(expectargv, actualargv)) {
virtTestDifference(stderr, expectargv, actualargv);
9:04 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #4
On Fri, Aug 08, 2008 at 02:07:49PM +0200, Jim Meyering wrote:
Guido Günther <agx(a)sigxcpu.org> wrote:
[..snip..]
> +static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr
dom, virDomainDeviceDefPtr dev)
> +{
> + struct qemud_driver *driver = (struct qemud_driver
*)dom->conn->privateData;
> + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
This should handle the case in which vm == NULL.
Otherwise, qemudMonitorCommand will dereference NULL.
...
The function gets called from qemudDomainAttachDevice which does the
same call to virDomainFindbyUUid and checks for vm == NULL already but
you're right: better safe than sorry.
[..snip..]
Please add a newline and split the long line:
/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi \
-boot c -hda /dev/HostVG/QEMUGuest1 -usbdevice disk:/tmp/usbdisk.img \
-net none -serial none -parallel none -usb
I'd rather not since this breaks
the testcase, it compares the exact
output.
New patches attached. Patches apply on top of the hostdev patches.
-- Guido
9:05 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #4
On Fri, Aug 08, 2008 at 02:30:15PM +0200, Jim Meyering wrote:
Guido Günther <agx(a)sigxcpu.org> wrote:
[..snip..]
> +static int qemudDomainAttachCdromDevice(virDomainPtr dom,
> + virDomainDeviceDefPtr dev)
> +{
> + struct qemud_driver *driver = (struct qemud_driver
*)dom->conn->privateData;
> + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
> + virDomainDiskDefPtr disk;
> +
> + disk = vm->def->disks;
Check for vm == NULL before dereferencing it.
...
> +static int qemudDomainAttachHostDevice(virDomainPtr dom, virDomainDeviceDefPtr
dev)
> +{
> + struct qemud_driver *driver = (struct qemud_driver
*)dom->conn->privateData;
> + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
...
> + if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
Likewise.
The same applies here: qemudDomainAttachDevice checked that already, but
again - better safe than sorry. Updated patches attached.
-- Guido
9:34 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #4
On Fri, Aug 08, 2008 at 04:05:10PM +0200, Guido Günther wrote:
> Likewise.
The same applies here: qemudDomainAttachDevice checked that already, but
again - better safe than sorry. Updated patches attached.
Okidoc, all look good ! Applied and commited to CVS,
thanks a lot for the work,
I think the only thing missing is extending the descrition in the documentation
would you mind adding a description in formatdomain.html(.in)
Probably an "USB devices" section added below the elementsDisks part,
explaining the syntaxes and stating that it was added in versions after 0.4.4
If you don't feel comfortable with that I can do it, but you know better :)
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
9:34 a.m.
New subject: [libvirt] [PATCH] append a newline to expected-output files lacking NL-at-EOF
Jim Meyering <jim(a)meyering.net> wrote:
Maybe you're talking about something else?
I proposed to add a trailing newline in the expected output
as well as in the code to generate the "actual" output.
This is the sort of change I was thinking of:
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
At first I thought that might be incomplete.
But it's all that's needed on the C side.
I ran this one-liner to append the missing newlines, and we're all set:
for i in $(find|grep '\.args$'); do echo >> $i;done
"make check" and "make syntax-check" both pass.
From b548c2f7caf311b7acfa2ebd839067f674d89a40 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 8 Aug 2008 16:18:57 +0200
Subject: [PATCH] tests: append a newline to expected-output files lacking NL-at-EOF
* tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Adjust the
code that creates "actual" output, so that it too produces a
newline-terminated buffer.
* tests/qemuxml2argvdata/*.args: Append a newline to each, via:
for i in $(find|grep '\.args$'); do echo >> $i;done
---
.../qemuxml2argvdata/qemuxml2argv-boot-cdrom.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-boot-floppy.args | 2 +-
.../qemuxml2argv-boot-network.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-bootloader.args | 2 +-
.../qemuxml2argv-clock-localtime.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args | 2 +-
.../qemuxml2argv-console-compat.args | 2 +-
.../qemuxml2argv-disk-cdrom-empty.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-cdrom.args | 2 +-
.../qemuxml2argv-disk-drive-boot-cdrom.args | 2 +-
.../qemuxml2argv-disk-drive-boot-disk.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-floppy.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-many.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-virtio.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args | 2 +-
.../qemuxml2argv-graphics-sdl.args | 2 +-
.../qemuxml2argv-graphics-vnc.args | 2 +-
.../qemuxml2argv-input-usbmouse.args | 2 +-
.../qemuxml2argv-input-usbtablet.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args | 2 +-
.../qemuxml2argv-misc-no-reboot.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-net-virtio.args | 2 +-
.../qemuxml2argv-parallel-tcp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-dev.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-file.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-many.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-pty.args | 2 +-
.../qemuxml2argv-serial-tcp-telnet.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-tcp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-udp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-unix.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-serial-vc.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-sound.args | 2 +-
tests/qemuxml2argvtest.c | 3 ++-
37 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args
b/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args
index 3f45b24..6c062ae 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args
@@ -1 +1 @@
-/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot d -cdrom
/dev/cdrom -net none -serial none -parallel none -usb
\ No newline at end of file
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot d -cdrom
/dev/cdrom -net none -serial none -parallel none -usb
...
[lots of generated changes elided]
...
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound.args
@@ -1 +1 @@
-/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -soundhw
pcspk,es1370,sb16
\ No newline at end of file
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -hda
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -soundhw
pcspk,es1370,sb16
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d136a13..25465ae 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -53,7 +53,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *cmd,
int extra
goto fail;
tmp = argv;
- len = 0;
+ len = 1; /* for trailing newline */
while (*tmp) {
len += strlen(*tmp) + 1;
tmp++;
@@ -68,6 +68,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *cmd,
int extra
strcat(actualargv, *tmp);
tmp++;
}
+ strcat(actualargv, "\n");
if (STRNEQ(expectargv, actualargv)) {
virtTestDifference(stderr, expectargv, actualargv);
--
1.6.0.rc2.2.g59bf
10:01 a.m.
New subject: [libvirt] [PATCH]: file backed usb massstorage #4
On Fri, Aug 08, 2008 at 04:04:17PM +0200, Guido Günther wrote:
New patches attached. Patches apply on top of the hostdev patches.
All commited, I just removed an extra space at end of line :-)
thanks a lot !
Again a small patch to the doc describing it would fix the last weak spot,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
10:59 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #4
Hi,
On Fri, Aug 08, 2008 at 10:34:00AM -0400, Daniel Veillard wrote:
[..snip..]
I think the only thing missing is extending the descrition in the
documentation
would you mind adding a description in formatdomain.html(.in)
Probably an "USB devices" section added below the elementsDisks part,
explaining the syntaxes and stating that it was added in versions after 0.4.4
If you don't feel comfortable with that I can do it, but you know better :)
Attached is a short patch for this. This might be a bit terse but we'll
have to change this anyhow once pci passthrough for xen arrives.
Cheers,
-- Guido
7:30 a.m.
New subject: [libvirt] [PATCH] append a newline to expected-output files lacking NL-at-EOF
Jim Meyering <jim(a)meyering.net> wrote:
Jim Meyering <jim(a)meyering.net> wrote:
> Maybe you're talking about something else?
> I proposed to add a trailing newline in the expected output
> as well as in the code to generate the "actual" output.
>
> This is the sort of change I was thinking of:
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
At first I thought that might be incomplete.
But it's all that's needed on the C side.
I ran this one-liner to append the missing newlines, and we're all set:
for i in $(find|grep '\.args$'); do echo >> $i;done
"make check" and "make syntax-check" both pass.
Planning to commit that, I rebased against the latest, but found "make
check" then failed due to three new expected-output ".args" files without
the now-required trailing newline. I added them, and now "make check"
passes, so I've just committed it.
2:29 a.m.
New subject: [libvirt] [PATCH]: hostdev passthrough support take #4
On Fri, Aug 08, 2008 at 05:59:56PM +0200, Guido Günther wrote:
Hi,
On Fri, Aug 08, 2008 at 10:34:00AM -0400, Daniel Veillard wrote:
[..snip..]
> I think the only thing missing is extending the descrition in the documentation
> would you mind adding a description in formatdomain.html(.in)
> Probably an "USB devices" section added below the elementsDisks part,
> explaining the syntaxes and stating that it was added in versions after 0.4.4
>
> If you don't feel comfortable with that I can do it, but you know better :)
Attached is a short patch for this. This might be a bit terse but we'll
have to change this anyhow once pci passthrough for xen arrives.
Cheers,
That's fine, just fixed a couple of typos and commited,
thanks a lot :-) !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
5902
days inactive
5920
days old
35 comments
5 participants
participants (5)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Guido Günther -
Jim Meyering