Jim Meyering wrote:
> Cole Robinson <crobinso@xxxxxxxxxx> wrote:
> ...
>> Okay, second cut attached. I followed your recommendations:
> ...
>
> With all of Dan's comments addressed, this should be fine.
>
>> diff --git a/src/domain_conf.c b/src/domain_conf.c
> ...
>> +int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
> ...
>> + switch (disk->bus) {
>> + case VIR_DOMAIN_DISK_BUS_IDE:
>> + *busIdx = ((idx - (idx % 2)) / 2);
>> + *devIdx = (idx % 2);
>> + break;
>> + case VIR_DOMAIN_DISK_BUS_SCSI:
>> + *busIdx = ((idx - (idx % 7)) / 7);
>> + *devIdx = (idx % 7);
>> + break;
>
> It doesn't affect correctness, but you can remove the
> unnecessary "- (idx % 2)" and "- (idx % 7)" expressions:
>
> switch (disk->bus) {
> case VIR_DOMAIN_DISK_BUS_IDE:
> *busIdx = idx / 2;
> *devIdx = idx % 2;
> break;
> case VIR_DOMAIN_DISK_BUS_SCSI:
> *busIdx = idx / 7;
> *devIdx = idx % 7;
> break;
Okay, latest cut. This addresses the above piece pointed out
by Jim and all of Dan's comments.
Thanks,
Cole
diff --git a/src/domain_conf.c b/src/domain_conf.c
index dc5eb0c..42f914f 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -3411,5 +3411,42 @@ char *virDomainConfigFile(virConnectPtr conn,
return ret;
}
+/* Translates a device name of the form (regex) "[fhv]d[a-z]+" into
+ * the corresponding bus,index combination (e.g. sda => (0,0), sdi (1,1),
+ * hdd => (1,1), vdaa => (0,27))
+ * @param disk The disk device
+ * @param busIdx parsed bus number
+ * @param devIdx parsed device number
+ * @return 0 on success, -1 on failure
+ */
+int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
+ int *busIdx,
+ int *devIdx) {
+
+ int idx = virDiskNameToIndex(disk->dst);
+ if (idx < 1)
+ return -1;
+
+ switch (disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ *busIdx = idx / 2;
+ *devIdx = idx % 2;
+ break;
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ *busIdx = idx / 7;
+ *devIdx = idx % 7;
+ break;
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ case VIR_DOMAIN_DISK_BUS_USB:
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ case VIR_DOMAIN_DISK_BUS_XEN:
+ default:
+ *busIdx = 0;
+ *devIdx = idx;
+ break;
+ }
+
+ return 0;
+}
#endif /* ! PROXY */
diff --git a/src/domain_conf.h b/src/domain_conf.h
index cfa2a90..de8a043 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -555,6 +555,10 @@ char *virDomainConfigFile(virConnectPtr conn,
const char *dir,
const char *name);
+int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
+ int *busIdx,
+ int *devIdx);
+
virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
xmlNodePtr node);
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ad14d02..e75a686 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2943,36 +2943,137 @@ static int qemudDomainUndefine(virDomainPtr dom) {
return 0;
}
-static int qemudDomainChangeCDROM(virDomainPtr dom,
- virDomainObjPtr vm,
- virDomainDiskDefPtr olddisk,
- virDomainDiskDefPtr newdisk) {
+/* Return the disks name for use in monitor commands */
+static char *qemudDiskDeviceName(virDomainPtr dom,
+ virDomainDiskDefPtr disk) {
+
+ int busid, devid;
+ int ret;
+ char *devname;
+
+ if (virDiskNameToBusDeviceIndex(disk, &busid, &devid) < 0) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot convert disk '%s' to bus/device index"),
+ disk->dst);
+ return NULL;
+ }
+
+ switch (disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ ret = asprintf(&devname, "ide%d-cd%d", busid, devid);
+ break;
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ ret = asprintf(&devname, "scsi%d-cd%d", busid, devid);
+ break;
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ ret = asprintf(&devname, "floppy%d", devid);
+ break;
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ ret = asprintf(&devname, "virtio%d", devid);
+ break;
+ default:
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _("Unsupported disk name mapping for bus '%s'"),
+ virDomainDiskBusTypeToString(disk->bus));
+ return NULL;
+ }
+
+ if (ret == -1) {
+ qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ return NULL;
+ }
+
+ return devname;
+}
+
+static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
+ virDomainDeviceDefPtr dev)
+{
struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ virDomainDiskDefPtr origdisk, newdisk;
char *cmd, *reply, *safe_path;
+ char *devname = NULL;
+ int qemuCmdFlags;
+
+ if (!vm) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
+ "%s", _("no domain with matching uuid"));
+ return -1;
+ }
+
+ newdisk = dev->data.disk;
+ origdisk = vm->def->disks;
+ while (origdisk) {
+ if (origdisk->bus == newdisk->bus &&
+ STREQ(origdisk->dst, newdisk->dst))
+ break;
+ origdisk = origdisk->next;
+ }
+
+ if (!origdisk) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("No device with bus '%s' and target '%s'"),
+ virDomainDiskBusTypeToString(newdisk->bus),
+ newdisk->dst);
+ return -1;
+ }
+
+ if (qemudExtractVersionInfo(vm->def->emulator,
+ NULL,
+ &qemuCmdFlags) < 0) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Cannot determine QEMU argv syntax %s"),
+ vm->def->emulator);
+ return -1;
+ }
+
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE) {
+ if (!(devname = qemudDiskDeviceName(dom, newdisk)))
+ return -1;
+ } else {
+ /* Back compat for no -drive option */
+ if (newdisk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ devname = strdup(newdisk->dst);
+ else if (newdisk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
+ STREQ(newdisk->dst, "hdc"))
+ devname = strdup("cdrom");
+ else {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Emulator version does not support removable "
+ "media for device '%s' and target '%s'"),
+ virDomainDiskDeviceTypeToString(newdisk->device),
+ newdisk->dst);
+ return -1;
+ }
+
+ if (!devname) {
+ qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ return -1;
+ }
+ }
if (newdisk->src) {
safe_path = qemudEscapeMonitorArg(newdisk->src);
if (!safe_path) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+ VIR_FREE(devname);
return -1;
}
- if (asprintf (&cmd, "change %s \"%s\"",
- /* XXX qemu may support multiple CDROM in future */
- /* olddisk->dst */ "cdrom",
- safe_path) == -1) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ if (asprintf (&cmd, "change %s \"%s\"", devname, safe_path) == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
VIR_FREE(safe_path);
+ VIR_FREE(devname);
return -1;
}
VIR_FREE(safe_path);
- } else if (asprintf(&cmd, "eject cdrom") == -1) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ } else if (asprintf(&cmd, "eject %s", devname) == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+ VIR_FREE(devname);
return -1;
}
+ VIR_FREE(devname);
if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
@@ -2984,7 +3085,7 @@ static int qemudDomainChangeCDROM(virDomainPtr dom,
/* If the command failed qemu prints:
* device not found, device is locked ...
* No message is printed on success it seems */
- DEBUG ("cdrom change reply: %s", reply);
+ DEBUG ("ejectable media change reply: %s", reply);
if (strstr(reply, "\ndevice ")) {
qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
"%s", _("changing cdrom media failed"));
@@ -2992,49 +3093,16 @@ static int qemudDomainChangeCDROM(virDomainPtr dom,
VIR_FREE(cmd);
return -1;
}
-
VIR_FREE(reply);
VIR_FREE(cmd);
- VIR_FREE(olddisk->src);
- olddisk->src = newdisk->src;
+ VIR_FREE(origdisk->src);
+ origdisk->src = newdisk->src;
newdisk->src = NULL;
- olddisk->type = newdisk->type;
+ origdisk->type = newdisk->type;
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;
-
- if (!vm) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
- "%s", _("no domain with matching uuid"));
- return -1;
- }
-
- disk = vm->def->disks;
- while (disk) {
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
- STREQ(disk->dst, dev->data.disk->dst))
- break;
- disk = disk->next;
- }
-
- if (!disk) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
- "%s", _("CDROM not attached, cannot change media"));
- return -1;
- }
-
- if (qemudDomainChangeCDROM(dom, vm, disk, dev->data.disk) < 0) {
- return -1;
- }
- return 0;
-}
static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr dev)
{
@@ -3186,10 +3254,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
}
if (dev->type == VIR_DOMAIN_DEVICE_DISK &&
- dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
- ret = qemudDomainAttachCdromDevice(dom, dev);
+ (dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+ dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) {
+ ret = qemudDomainChangeEjectableMedia(dom, dev);
} else if (dev->type == VIR_DOMAIN_DEVICE_DISK &&
- dev->data.disk->device == VIR_DOMAIN_DEVICE_DISK &&
+ dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
ret = qemudDomainAttachUsbMassstorageDevice(dom, dev);
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list