External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-01 01:15:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't blindly reorder disk drives

Browse Source 
Calling qsort() on the disks array causes disk to be
unneccessarily re-ordered, potentially breaking the
ability to boot if the boot disk gets moved later in
the list. The new algorithm will insert a new disk as
far to the end of the list as possible, while being
ordered correctly wrt other disks on the same bus.

* src/domain_conf.c, src/domain_conf.h: Remove disk sorting
  routines. Add API to insert a disk into existing list at
  the optimal position, without resorting disks
* src/libvirt_private.syms: Export virDomainDiskInsert
* src/xend_internal.c, src/xm_internal.c: Remove calls to
  qsort, use virDomainDiskInsert instead.
* src/qemu_driver.c: Remove calls to qsort, use virDoaminDiskInsert
  instead. Fix reordering bugs when hotunplugging disks and
  networks. Fix memory leak in disk/net unplug
This commit is contained in:
Daniel P. Berrange 2009-08-14 10:31:36 +01:00
parent fea5a0bdc9
commit 2d6adabd53
9 changed files with 96 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/domain_conf.c
View File

@ -631,19 +631,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
}
}
#endif /* ! PROXY */
int virDomainDiskCompare(virDomainDiskDefPtr a,
virDomainDiskDefPtr b) {
if (a->bus == b->bus)
return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst);
else
return a->bus - b->bus;
}
#ifndef PROXY
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@ -2343,14 +2332,61 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn,
}
#endif
int virDomainDiskQSort(const void *a, const void *b)
{
const virDomainDiskDefPtr *da = a;
const virDomainDiskDefPtr *db = b;
return virDomainDiskCompare(*da, *db);
int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0)
return -1;
virDomainDiskInsertPreAlloced(def, disk);
return 0;
}
void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
int i;
/* Tenatively plan to insert disk at the end. */
int insertAt = -1;
/* Then work backwards looking for disks on
* the same bus. If we find a disk with a drive
* index greater than the new one, insert at
* that position
*/
for (i = (def->ndisks - 1) ; i >= 0 ; i--) {
/* If bus matches and current disk is after
* new disk, then new disk should go here */
if (def->disks[i]->bus == disk->bus &&
(virDiskNameToIndex(def->disks[i]->dst) >
virDiskNameToIndex(disk->dst))) {
insertAt = i;
} else if (def->disks[i]->bus == disk->bus &&
insertAt == -1) {
/* Last disk with match bus is before the
* new disk, then put new disk just after
*/
insertAt = i + 1;
}
}
/* No disks with this bus yet, so put at end of list */
if (insertAt == -1)
insertAt = def->ndisks;
if (insertAt < def->ndisks)
memmove(def->disks + insertAt + 1,
def->disks + insertAt,
(sizeof(def->disks[0]) * (def->ndisks-insertAt)));
def->disks[insertAt] = disk;
def->ndisks++;
}
#ifndef PROXY
static char *virDomainDefDefaultEmulator(virConnectPtr conn,
virDomainDefPtr def,
@ -2657,8 +2693,6 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
def->disks[def->ndisks++] = disk;
}
qsort(def->disks, def->ndisks, sizeof(*def->disks),
virDomainDiskQSort);
VIR_FREE(nodes);
/* analysis of the filesystems */

7
src/domain_conf.h
View File

@ -685,9 +685,10 @@ char *virDomainCpuSetFormat(virConnectPtr conn,
char *cpuset,
int maxcpu);
int virDomainDiskQSort(const void *a, const void *b);
int virDomainDiskCompare(virDomainDiskDefPtr a,
virDomainDiskDefPtr b);
int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk);
void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
virDomainDiskDefPtr disk);
int virDomainSaveXML(virConnectPtr conn,
const char *configDir,

3
src/libvirt_private.syms
View File

@ -82,7 +82,8 @@ virDomainDeviceTypeToString;
virDomainDiskBusTypeToString;
virDomainDiskDefFree;
virDomainDiskDeviceTypeToString;
virDomainDiskQSort;
virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
virDomainFindByID;
virDomainFindByName;
virDomainFindByUUID;

50
src/qemu_driver.c
View File

@ -5169,9 +5169,7 @@ try_command:
dev->data.disk->pci_addr.bus = bus;
dev->data.disk->pci_addr.slot = slot;
vm->def->disks[vm->def->ndisks++] = dev->data.disk;
qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks),
virDomainDiskQSort);
virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
return 0;
}
@ -5229,9 +5227,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
return -1;
}
vm->def->disks[vm->def->ndisks++] = dev->data.disk;
qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks),
virDomainDiskQSort);
virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
VIR_FREE(reply);
VIR_FREE(cmd);
@ -5772,16 +5768,20 @@ try_command:
goto cleanup;
}
if (i != --vm->def->ndisks)
memmove(&vm->def->disks[i],
&vm->def->disks[i+1],
sizeof(*vm->def->disks) * (vm->def->ndisks-i));
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) {
virReportOOMError(conn);
goto cleanup;
if (vm->def->ndisks > 1) {
memmove(vm->def->disks + i,
vm->def->disks + i + 1,
sizeof(*vm->def->disks) *
(vm->def->ndisks - (i + 1)));
vm->def->ndisks--;
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) {
/* ignore, harmless */
}
} else {
VIR_FREE(vm->def->disks[0]);
vm->def->ndisks = 0;
}
qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks),
virDomainDiskQSort);
virDomainDiskDefFree(detach);
ret = 0;
@ -5870,14 +5870,20 @@ qemudDomainDetachNetDevice(virConnectPtr conn,
DEBUG("%s: host_net_remove reply: %s", vm->def->name, reply);
if (i != --vm->def->nnets)
memmove(&vm->def->nets[i],
&vm->def->nets[i+1],
sizeof(*vm->def->nets) * (vm->def->nnets-i));
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) {
virReportOOMError(conn);
goto cleanup;
if (vm->def->nnets > 1) {
memmove(vm->def->nets + i,
vm->def->nets + i + 1,
sizeof(*vm->def->nets) *
(vm->def->nnets - (i + 1)));
vm->def->nnets--;
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) {
/* ignore, harmless */
}
} else {
VIR_FREE(vm->def->nets[0]);
vm->def->nnets = 0;
}
virDomainNetDefFree(detach);
ret = 0;

2
src/xend_internal.c
View File

@ -2540,8 +2540,6 @@ xenDaemonParseSxpr(virConnectPtr conn,
}
}
}
qsort(def->disks, def->ndisks, sizeof(*def->disks),
virDomainDiskQSort);
/* in case of HVM we have USB device emulation */
if (hvm &&

7
src/xm_internal.c
View File

@ -990,8 +990,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
disk = NULL;
}
}
qsort(def->disks, def->ndisks, sizeof(*def->disks),
virDomainDiskQSort);
list = virConfGetValue(conf, "vif");
if (list && list->type == VIR_CONF_LIST) {
@ -2839,14 +2837,11 @@ xenXMDomainAttachDevice(virDomainPtr domain, const char *xml) {
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
{
if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) {
if (virDomainDiskInsert(def, dev->data.disk) < 0) {
virReportOOMError(domain->conn);
goto cleanup;
}
def->disks[def->ndisks++] = dev->data.disk;
dev->data.disk = NULL;
qsort(def->disks, def->ndisks, sizeof(*def->disks),
virDomainDiskQSort);
}
break;

2
tests/sexpr2xmldata/sexpr2xml-fv-v2.sexpr
View File

@ -1 +1 @@
(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib64/xen/bin/qemu-dm')(boot c)(acpi 1)(vnc 1)(keymap ja)))(device (vbd (dev 'hdc:cdrom')(uname 'file:/root/boot.iso')(mode 'r')))(device (vbd (dev 'hda:disk')(uname 'file:/root/foo.img')(mode 'w')))(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu))))
(domain (domid 3)(name 'fvtest')(memory 400)(maxmem 400)(vcpus 1)(uuid 'b5d70dd275cdaca517769660b059d8bc')(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib64/xen/bin/qemu-dm')(boot c)(acpi 1)(vnc 1)(keymap ja)))(device (vbd (dev 'hda:disk')(uname 'file:/root/foo.img')(mode 'w')))(device (vbd (dev 'hdc:cdrom')(uname 'file:/root/boot.iso')(mode 'r')))(device (vif (mac '00:16:3e:1b:b1:47')(bridge 'xenbr0')(script 'vif-bridge')(type ioemu))))

8
tests/xml2sexprdata/xml2sexpr-fv-vncunused.xml
View File

@ -21,15 +21,15 @@
<mac address='00:16:3e:1b:b1:47'/>
<script path='vif-bridge'/>
</interface>
<disk type='file'>
<source file='/root/foo.img'/>
<target dev='ioemu:hda'/>
</disk>
<disk type='file' device='cdrom'>
<source file='/root/boot.iso'/>
<target dev='hdc'/>
<readonly/>
</disk>
<disk type='file'>
<source file='/root/foo.img'/>
<target dev='ioemu:hda'/>
</disk>
<graphics type='vnc' port='-1' keymap='ja'/>
</devices>
</domain>

8
tests/xml2sexprdata/xml2sexpr-fv.xml
View File

@ -21,15 +21,15 @@
<mac address='00:16:3e:1b:b1:47'/>
<script path='vif-bridge'/>
</interface>
<disk type='file'>
<source file='/root/foo.img'/>
<target dev='ioemu:hda'/>
</disk>
<disk type='file' device='cdrom'>
<source file='/root/boot.iso'/>
<target dev='hdc'/>
<readonly/>
</disk>
<disk type='file'>
<source file='/root/foo.img'/>
<target dev='ioemu:hda'/>
</disk>
<graphics type='vnc' port='5917' keymap='ja'/>
</devices>
</domain>