[libvirt] [PATCH v2] xen: Prevent updating device when attaching a device
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device. Changes from v1 to v2: - allow update CDROM * src/xen/xend_internal.c --- src/xen/xend_internal.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index cd30336..bc23595 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3965,6 +3965,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; + char *target = NULL; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4029,6 +4030,13 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1) < 0) goto cleanup; + + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + if (!(target = strdup(dev->data.disk->dst))) { + virReportOOMError(); + goto cleanup; + } + } break; case VIR_DOMAIN_DEVICE_NET: @@ -4038,6 +4046,14 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1) < 0) goto cleanup; + + char macStr[VIR_MAC_STRING_BUFLEN]; + virFormatMacAddr(dev->data.net->mac, macStr); + + if (!(target = strdup(macStr))) { + virReportOOMError(); + goto cleanup; + } break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -4046,6 +4062,17 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, if (xenDaemonFormatSxprOnePCI(dev->data.hostdev, &buf, 0) < 0) goto cleanup; + + virDomainDevicePCIAddress PCIAddr; + + PCIAddr = dev->data.hostdev->source.subsys.u.pci; + virAsprintf(&target, "PCI device: %.4x:%.2x:%.2x", PCIAddr.domain, + PCIAddr.bus, PCIAddr.slot); + + if (target == NULL) { + virReportOOMError(); + goto cleanup; + } } else { virXendError(VIR_ERR_NO_SUPPORT, "%s", _("unsupported device type")); @@ -4065,17 +4092,22 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, /* device doesn't exist, define it */ ret = xend_op(domain->conn, domain->name, "op", "device_create", "config", sexpr, NULL); - } - else { - /* device exists, attempt to modify it */ - ret = xend_op(domain->conn, domain->name, "op", "device_configure", - "config", sexpr, "dev", ref, NULL); + } else { + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + virXendError(VIR_ERR_OPERATION_INVALID, + _("target '%s' already exists"), target); + } else { + /* device exists, attempt to modify it */ + ret = xend_op(domain->conn, domain->name, "op", "device_configure", + "config", sexpr, "dev", ref, NULL); + } } cleanup: VIR_FREE(sexpr); virDomainDefFree(def); virDomainDeviceDefFree(dev); + VIR_FREE(target); return ret; } -- 1.7.3.2
于 2011年02月09日 16:51, Osier Yang 写道:
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device.
Changes from v1 to v2: - allow update CDROM
s/allow/allow to/ will change it when pushing. :-) Regards Osier
On Wed, Feb 09, 2011 at 04:51:27PM +0800, Osier Yang wrote:
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device.
Changes from v1 to v2: - allow update CDROM
* src/xen/xend_internal.c --- src/xen/xend_internal.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index cd30336..bc23595 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3965,6 +3965,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; + char *target = NULL;
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4029,6 +4030,13 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1) < 0) goto cleanup; + + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
I can't remember if Xen supports it or not, but do we need DEVICE_FLOPPY here too ? The patch looks good aside from that question Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Feb 09, 2011 at 02:01:25PM +0000, Daniel P. Berrange wrote:
On Wed, Feb 09, 2011 at 04:51:27PM +0800, Osier Yang wrote:
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device.
Changes from v1 to v2: - allow update CDROM
* src/xen/xend_internal.c --- src/xen/xend_internal.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index cd30336..bc23595 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3965,6 +3965,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; + char *target = NULL;
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4029,6 +4030,13 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1) < 0) goto cleanup; + + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
I can't remember if Xen supports it or not, but do we need DEVICE_FLOPPY here too ?
I would guess that yes, we use it in the xend driver
The patch looks good aside from that question
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
于 2011年02月10日 14:09, Daniel Veillard 写道:
On Wed, Feb 09, 2011 at 02:01:25PM +0000, Daniel P. Berrange wrote:
On Wed, Feb 09, 2011 at 04:51:27PM +0800, Osier Yang wrote:
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device.
Changes from v1 to v2: - allow update CDROM
* src/xen/xend_internal.c --- src/xen/xend_internal.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index cd30336..bc23595 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3965,6 +3965,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; + char *target = NULL;
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4029,6 +4030,13 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1)< 0) goto cleanup; + + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
I can't remember if Xen supports it or not, but do we need DEVICE_FLOPPY here too ?
I would guess that yes, we use it in the xend driver
The patch looks good aside from that question
Daniel
Checked xend's code, from the code, floppy device updating is not supported, and also tried to update a floppy device via "xm", that's true. file: "xend/server/blkif.py" def reconfigureDevice(self, _, config): """@see DevController.reconfigureDevice""" (devid, new_back, new_front) = self.getDeviceDetails(config) (dev, mode) = self.readBackend(devid, 'dev', 'mode') dev_type = self.readFrontend(devid, 'device-type') if (dev_type == 'cdrom' and new_front['device-type'] == 'cdrom' and dev == new_back['dev'] and mode == 'r'): if not os.access(new_back['params'],os.R_OK): raise VmError("Can't read disk file %s" % new_back['params']) self.writeBackend(devid, 'type', new_back['type'], 'params', new_back['params']) else: raise VmError('Refusing to reconfigure device %s:%d to %s' % (self.deviceClass, devid, config)) [root@dhcp exp]# xm block-list RHEL5-hvm-64 Vdev BE handle state evt-ch ring-ref BE-path 51712 0 0 1 -1 -1 /local/domain/0/backend/tap/18/51712 4058 0 0 1 -1 -1 /local/domain/0/backend/vbd/18/4058 [root@dhcp- exp]# xm block-configure 18 file:/var/lib/xen/images/floppy.img fda:floppy r Error: Refusing to reconfigure device vbd:4058 to ['vbd', ['uname', 'file:/var/lib/xen/images/floppy.img'], ['dev', 'fda:floppy'], ['mode', 'r']] so I will push the patch. Regards Osier
于 2011年02月11日 16:07, Osier Yang 写道:
于 2011年02月10日 14:09, Daniel Veillard 写道:
On Wed, Feb 09, 2011 at 02:01:25PM +0000, Daniel P. Berrange wrote:
On Wed, Feb 09, 2011 at 04:51:27PM +0800, Osier Yang wrote:
When attaching a device that already exists, xend driver updates the device with "device_configure", it causes problems (e.g. for disk device, 'device_configure' only can be used to update device like CDROM), on the other hand, we provide additional API (virDomainUpdateDevice) to update device, this fix is to raise up errors instead of updating the existed device which is not CDROM device.
Changes from v1 to v2: - allow update CDROM
* src/xen/xend_internal.c --- src/xen/xend_internal.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index cd30336..bc23595 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3965,6 +3965,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char class[8], ref[80]; + char *target = NULL;
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -4029,6 +4030,13 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, STREQ(def->os.type, "hvm") ? 1 : 0, priv->xendConfigVersion, 1)< 0) goto cleanup; + + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
I can't remember if Xen supports it or not, but do we need DEVICE_FLOPPY here too ?
I would guess that yes, we use it in the xend driver
The patch looks good aside from that question
Daniel
Checked xend's code, from the code, floppy device updating is not supported, and also tried to update a floppy device via "xm", that's true.
file: "xend/server/blkif.py"
def reconfigureDevice(self, _, config): """@see DevController.reconfigureDevice""" (devid, new_back, new_front) = self.getDeviceDetails(config)
(dev, mode) = self.readBackend(devid, 'dev', 'mode') dev_type = self.readFrontend(devid, 'device-type')
if (dev_type == 'cdrom' and new_front['device-type'] == 'cdrom' and dev == new_back['dev'] and mode == 'r'):
if not os.access(new_back['params'],os.R_OK): raise VmError("Can't read disk file %s" % new_back['params'])
self.writeBackend(devid, 'type', new_back['type'], 'params', new_back['params']) else: raise VmError('Refusing to reconfigure device %s:%d to %s' % (self.deviceClass, devid, config))
[root@dhcp exp]# xm block-list RHEL5-hvm-64 Vdev BE handle state evt-ch ring-ref BE-path 51712 0 0 1 -1 -1 /local/domain/0/backend/tap/18/51712 4058 0 0 1 -1 -1 /local/domain/0/backend/vbd/18/4058
[root@dhcp- exp]# xm block-configure 18 file:/var/lib/xen/images/floppy.img fda:floppy r Error: Refusing to reconfigure device vbd:4058 to ['vbd', ['uname', 'file:/var/lib/xen/images/floppy.img'], ['dev', 'fda:floppy'], ['mode', 'r']]
so I will push the patch.
Regards Osier
Pushed Regards, Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Osier Yang