2:59 p.m.
I don't think all snprintf uses are bad - those that format a number
into an appropriately-sized maximum-possible array should be fine.
For example, there are some fixed-size buffers whose size is determined
by the kernel, where snprintf was the appropriate course of action
in nwfilter_ebiptables_driver.c. But sprintf doesn't have quite the
coverage as snprintf, so this continuation of *printf cleanups converts
some of these.
I still have more patches to go, but this has taken me long enough
to get some more review rather than slamming a 20-patch series all
at the end.
More comments within individual patches.
Eric Blake (7):
build: add some modules
vbox: factor a large function
vbox: avoid problematic uses of sprintf
network: use virAsprintf when appropriate
lxc: avoid large stacks with veth creation
openvz: formatting cleanups
openvz: use virAsprintf to avoid large stacks
bootstrap.conf | 2 +
src/conf/network_conf.c | 15 +-
src/lxc/lxc_driver.c | 32 +-
src/lxc/veth.c | 84 ++-
src/lxc/veth.h | 6 +-
src/openvz/openvz_conf.c | 142 ++--
src/openvz/openvz_driver.c | 23 +-
src/vbox/vbox_tmpl.c | 2108 +++++++++++++++++++++++---------------------
8 files changed, 1269 insertions(+), 1143 deletions(-)
--
1.7.2.2
2:59 p.m.
New subject: [libvirt] [PATCH 1/7] build: add some modules
snprintf is currently implicitly picked up by getaddrinfo, but we
might as well make it explicit so that mingw doesn't break if
getaddrinfo changes to drop the dependency.
func doesn't matter for gcc compilation, but may help other compilers
cope with our use of __func__.
* bootstrap.conf (gnulib_modules): Add snprintf and func.
---
Shouldn't impact a GNU/Linux build.
bootstrap.conf | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index ca31a6e..8a85b91 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -29,6 +29,7 @@ count-one-bits
crypto/md5
dirname-lgpl
fcntl-h
+func
getaddrinfo
gethostname
getpass
@@ -53,6 +54,7 @@ random_r
sched
send
setsockopt
+snprintf
socket
stpcpy
strchrnul
--
1.7.2.2
4:04 p.m.
New subject: [libvirt] [PATCH 1/7] build: add some modules
2010/9/1 Eric Blake <eblake(a)redhat.com>:
snprintf is currently implicitly picked up by getaddrinfo, but we
might as well make it explicit so that mingw doesn't break if
getaddrinfo changes to drop the dependency.
func doesn't matter for gcc compilation, but may help other compilers
cope with our use of __func__.
* bootstrap.conf (gnulib_modules): Add snprintf and func.
---
Shouldn't impact a GNU/Linux build.
ACK.
Matthias
6:01 p.m.
New subject: [libvirt] [PATCH 1/7] build: add some modules
On 09/01/2010 03:04 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> snprintf is currently implicitly picked up by getaddrinfo, but we
> might as well make it explicit so that mingw doesn't break if
> getaddrinfo changes to drop the dependency.
>
> func doesn't matter for gcc compilation, but may help other compilers
> cope with our use of __func__.
>
> * bootstrap.conf (gnulib_modules): Add snprintf and func.
> ---
>
> Shouldn't impact a GNU/Linux build.
>
ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 2/7] vbox: factor a large function
* src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Split...
(vboxSetBootDeviceOrder, vboxAttachDrives, vboxAttachSound)
(vboxAttachNetwork, vboxAttachSerial, vboxAttachParallel)
(vboxAttachVideo, vboxAttachDisplay, vboxAttachUSB): ...into new
helper functions.
---
Should be no functional changes - just breaking up a HUGE function
into more manageable chunks with less indentation.
In reviewing this, it may help to apply the patch and use
'git diff -b'.
src/vbox/vbox_tmpl.c | 2083 ++++++++++++++++++++++++++------------------------
1 files changed, 1082 insertions(+), 1001 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3c07509..3e8ff23 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3379,1186 +3379,1267 @@ static int vboxDomainCreate(virDomainPtr dom) {
return vboxDomainCreateWithFlags(dom, 0);
}
-static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
- VBOX_OBJECT_CHECK(conn, virDomainPtr, NULL);
- IMachine *machine = NULL;
- IBIOSSettings *bios = NULL;
- vboxIID *iid = NULL;
- vboxIID *mchiid = NULL;
- virDomainDefPtr def = NULL;
- PRUnichar *machineNameUtf16 = NULL;
-#if VBOX_API_VERSION >= 3002
- PRBool override = PR_FALSE;
-#endif
- nsresult rc;
-
- if (!(def = virDomainDefParseString(data->caps, xml,
- VIR_DOMAIN_XML_INACTIVE))) {
- goto cleanup;
- }
-
-#if VBOX_API_VERSION == 2002
- if (VIR_ALLOC(iid) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-#endif
+static void
+vboxSetBootDeviceOrder(virDomainDefPtr def, vboxGlobalData *data,
+ IMachine *machine)
+{
+ ISystemProperties *systemProperties = NULL;
+ PRUint32 maxBootPosition = 0;
+ int i = 0;
- VBOX_UTF8_TO_UTF16(def->name, &machineNameUtf16);
- vboxIIDFromUUID(def->uuid, iid);
-#if VBOX_API_VERSION < 3002
- rc = data->vboxObj->vtbl->CreateMachine(data->vboxObj,
- machineNameUtf16,
- NULL,
- NULL,
- iid,
- &machine);
-#else /* VBOX_API_VERSION >= 3002 */
- rc = data->vboxObj->vtbl->CreateMachine(data->vboxObj,
- machineNameUtf16,
- NULL,
- NULL,
- iid,
- override,
- &machine);
-#endif /* VBOX_API_VERSION >= 3002 */
- VBOX_UTF16_FREE(machineNameUtf16);
+ DEBUG("def->os.type %s", def->os.type);
+ DEBUG("def->os.arch %s", def->os.arch);
+ DEBUG("def->os.machine %s", def->os.machine);
+ DEBUG("def->os.nBootDevs %d", def->os.nBootDevs);
+ DEBUG("def->os.bootDevs[0] %d", def->os.bootDevs[0]);
+ DEBUG("def->os.bootDevs[1] %d", def->os.bootDevs[1]);
+ DEBUG("def->os.bootDevs[2] %d", def->os.bootDevs[2]);
+ DEBUG("def->os.bootDevs[3] %d", def->os.bootDevs[3]);
+ DEBUG("def->os.init %s", def->os.init);
+ DEBUG("def->os.kernel %s", def->os.kernel);
+ DEBUG("def->os.initrd %s", def->os.initrd);
+ DEBUG("def->os.cmdline %s", def->os.cmdline);
+ DEBUG("def->os.root %s", def->os.root);
+ DEBUG("def->os.loader %s", def->os.loader);
+ DEBUG("def->os.bootloader %s", def->os.bootloader);
+ DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not define a domain, rc=%08x"), (unsigned)rc);
- goto cleanup;
+ data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
+ if (systemProperties) {
+ systemProperties->vtbl->GetMaxBootPosition(systemProperties,
+ &maxBootPosition);
+ VBOX_RELEASE(systemProperties);
+ systemProperties = NULL;
}
- rc = machine->vtbl->SetMemorySize(machine, def->memory / 1024);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not set the memory size of the domain to: %lu Kb, "
- "rc=%08x"),
- def->memory, (unsigned)rc);
+ /* Clear the defaults first */
+ for (i = 0; i < maxBootPosition; i++) {
+ machine->vtbl->SetBootOrder(machine, i+1, DeviceType_Null);
}
- rc = machine->vtbl->SetCPUCount(machine, def->vcpus);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not set the number of virtual CPUs to: %lu,
rc=%08x"),
- def->vcpus, (unsigned)rc);
- }
+ for (i = 0; (i < def->os.nBootDevs) && (i < maxBootPosition); i++)
{
+ PRUint32 device = DeviceType_Null;
-#if VBOX_API_VERSION < 3001
- rc = machine->vtbl->SetPAEEnabled(machine, (def->features) &
- (1 << VIR_DOMAIN_FEATURE_PAE));
-#elif VBOX_API_VERSION == 3001
- rc = machine->vtbl->SetCpuProperty(machine, CpuPropertyType_PAE,
- (def->features) &
- (1 << VIR_DOMAIN_FEATURE_PAE));
-#elif VBOX_API_VERSION >= 3002
- rc = machine->vtbl->SetCPUProperty(machine, CPUPropertyType_PAE,
- (def->features) &
- (1 << VIR_DOMAIN_FEATURE_PAE));
-#endif
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not change PAE status to: %s, rc=%08x"),
- ((def->features) & (1 << VIR_DOMAIN_FEATURE_PAE))
- ? _("Enabled") : _("Disabled"), (unsigned)rc);
- }
-
- machine->vtbl->GetBIOSSettings(machine, &bios);
- if (bios) {
- rc = bios->vtbl->SetACPIEnabled(bios, (def->features) &
- (1 << VIR_DOMAIN_FEATURE_ACPI));
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not change ACPI status to: %s, rc=%08x"),
- ((def->features) & (1 << VIR_DOMAIN_FEATURE_ACPI))
- ? _("Enabled") : _("Disabled"), (unsigned)rc);
- }
- rc = bios->vtbl->SetIOAPICEnabled(bios, (def->features) &
- (1 << VIR_DOMAIN_FEATURE_APIC));
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not change APIC status to: %s, rc=%08x"),
- ((def->features) & (1 << VIR_DOMAIN_FEATURE_APIC))
- ? _("Enabled") : _("Disabled"), (unsigned)rc);
+ if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_FLOPPY) {
+ device = DeviceType_Floppy;
+ } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_CDROM) {
+ device = DeviceType_DVD;
+ } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_DISK) {
+ device = DeviceType_HardDisk;
+ } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) {
+ device = DeviceType_Network;
}
- VBOX_RELEASE(bios);
- }
-
- /* Register the machine before attaching other devices to it */
- rc = data->vboxObj->vtbl->RegisterMachine(data->vboxObj, machine);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not define a domain, rc=%08x"), (unsigned)rc);
- goto cleanup;
+ machine->vtbl->SetBootOrder(machine, i+1, device);
}
+}
- /* Get the uuid of the machine, currently it is immutable
- * object so open a session to it and get it back, so that
- * you can make changes to the machine setting
- */
- machine->vtbl->GetId(machine, &mchiid);
- data->vboxObj->vtbl->OpenSession(data->vboxObj, data->vboxSession,
mchiid);
- data->vboxSession->vtbl->GetMachine(data->vboxSession, &machine);
-
- { /* Started:Block to set the boot device order */
- ISystemProperties *systemProperties = NULL;
- PRUint32 maxBootPosition = 0;
- int i = 0;
-
- DEBUG("def->os.type %s", def->os.type);
- DEBUG("def->os.arch %s", def->os.arch);
- DEBUG("def->os.machine %s", def->os.machine);
- DEBUG("def->os.nBootDevs %d", def->os.nBootDevs);
- DEBUG("def->os.bootDevs[0] %d", def->os.bootDevs[0]);
- DEBUG("def->os.bootDevs[1] %d", def->os.bootDevs[1]);
- DEBUG("def->os.bootDevs[2] %d", def->os.bootDevs[2]);
- DEBUG("def->os.bootDevs[3] %d", def->os.bootDevs[3]);
- DEBUG("def->os.init %s", def->os.init);
- DEBUG("def->os.kernel %s", def->os.kernel);
- DEBUG("def->os.initrd %s", def->os.initrd);
- DEBUG("def->os.cmdline %s", def->os.cmdline);
- DEBUG("def->os.root %s", def->os.root);
- DEBUG("def->os.loader %s", def->os.loader);
- DEBUG("def->os.bootloader %s", def->os.bootloader);
- DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs);
-
- data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
- if (systemProperties) {
- systemProperties->vtbl->GetMaxBootPosition(systemProperties,
&maxBootPosition);
- VBOX_RELEASE(systemProperties);
- systemProperties = NULL;
- }
-
- /* Clear the defaults first */
- for (i = 0; i < maxBootPosition; i++) {
- machine->vtbl->SetBootOrder(machine, i+1, DeviceType_Null);
- }
-
- for (i = 0; (i < def->os.nBootDevs) && (i < maxBootPosition);
i++) {
- PRUint32 device = DeviceType_Null;
-
- if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_FLOPPY) {
- device = DeviceType_Floppy;
- } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_CDROM) {
- device = DeviceType_DVD;
- } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_DISK) {
- device = DeviceType_HardDisk;
- } else if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) {
- device = DeviceType_Network;
- }
- machine->vtbl->SetBootOrder(machine, i+1, device);
- }
- } /* Finished:Block to set the boot device order */
+static void
+vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ int i;
+ nsresult rc;
#if VBOX_API_VERSION < 3001
- { /* Started:Block to attach the CDROM/DVD Drive and HardDisks to the VM */
-
- if (def->ndisks > 0) {
- int i;
-
- for (i = 0; i < def->ndisks; i++) {
- DEBUG("disk(%d) type: %d", i,
def->disks[i]->type);
- DEBUG("disk(%d) device: %d", i,
def->disks[i]->device);
- DEBUG("disk(%d) bus: %d", i, def->disks[i]->bus);
- DEBUG("disk(%d) src: %s", i, def->disks[i]->src);
- DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst);
- DEBUG("disk(%d) driverName: %s", i,
def->disks[i]->driverName);
- DEBUG("disk(%d) driverType: %s", i,
def->disks[i]->driverType);
- DEBUG("disk(%d) cachemode: %d", i,
def->disks[i]->cachemode);
- DEBUG("disk(%d) readonly: %s", i,
def->disks[i]->readonly ? "True" : "False");
- DEBUG("disk(%d) shared: %s", i, def->disks[i]->shared
? "True" : "False");
-
- if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
- if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE
&&
- def->disks[i]->src != NULL) {
- IDVDDrive *dvdDrive = NULL;
- /* Currently CDROM/DVD Drive is always IDE
- * Secondary Master so neglecting the following
- * parameters:
- * def->disks[i]->bus
- * def->disks[i]->dst
- */
+ if (def->ndisks == 0)
+ return;
- machine->vtbl->GetDVDDrive(machine, &dvdDrive);
- if (dvdDrive) {
- IDVDImage *dvdImage = NULL;
- PRUnichar *dvdfileUtf16 = NULL;
- vboxIID *dvduuid = NULL;
+ for (i = 0; i < def->ndisks; i++) {
+ DEBUG("disk(%d) type: %d", i, def->disks[i]->type);
+ DEBUG("disk(%d) device: %d", i, def->disks[i]->device);
+ DEBUG("disk(%d) bus: %d", i, def->disks[i]->bus);
+ DEBUG("disk(%d) src: %s", i, def->disks[i]->src);
+ DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst);
+ DEBUG("disk(%d) driverName: %s", i, def->disks[i]->driverName);
+ DEBUG("disk(%d) driverType: %s", i, def->disks[i]->driverType);
+ DEBUG("disk(%d) cachemode: %d", i, def->disks[i]->cachemode);
+ DEBUG("disk(%d) readonly: %s", i, (def->disks[i]->readonly
+ ? "True" : "False"));
+ DEBUG("disk(%d) shared: %s", i, (def->disks[i]->shared
+ ? "True" : "False"));
+
+ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
+ if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE &&
+ def->disks[i]->src != NULL) {
+ IDVDDrive *dvdDrive = NULL;
+ /* Currently CDROM/DVD Drive is always IDE
+ * Secondary Master so neglecting the following
+ * parameters:
+ * def->disks[i]->bus
+ * def->disks[i]->dst
+ */
+
+ machine->vtbl->GetDVDDrive(machine, &dvdDrive);
+ if (dvdDrive) {
+ IDVDImage *dvdImage = NULL;
+ PRUnichar *dvdfileUtf16 = NULL;
+ vboxIID *dvduuid = NULL;
# if VBOX_API_VERSION == 2002
- nsID dvdemptyuuid;
+ nsID dvdemptyuuid;
- memset(&dvdemptyuuid, 0, sizeof(dvdemptyuuid));
+ memset(&dvdemptyuuid, 0, sizeof(dvdemptyuuid));
# else
- PRUnichar *dvdemptyuuidUtf16 = NULL;
+ PRUnichar *dvdemptyuuidUtf16 = NULL;
# endif
- VBOX_UTF8_TO_UTF16(def->disks[i]->src,
&dvdfileUtf16);
+ VBOX_UTF8_TO_UTF16(def->disks[i]->src, &dvdfileUtf16);
- data->vboxObj->vtbl->FindDVDImage(data->vboxObj,
dvdfileUtf16, &dvdImage);
- if (!dvdImage) {
+ data->vboxObj->vtbl->FindDVDImage(data->vboxObj,
+ dvdfileUtf16, &dvdImage);
+ if (!dvdImage) {
# if VBOX_API_VERSION == 2002
-
data->vboxObj->vtbl->OpenDVDImage(data->vboxObj, dvdfileUtf16,
&dvdemptyuuid, &dvdImage);
+ data->vboxObj->vtbl->OpenDVDImage(data->vboxObj,
+ dvdfileUtf16,
+ &dvdemptyuuid,
+ &dvdImage);
# else
-
data->vboxObj->vtbl->OpenDVDImage(data->vboxObj, dvdfileUtf16,
dvdemptyuuidUtf16, &dvdImage);
+ data->vboxObj->vtbl->OpenDVDImage(data->vboxObj,
+ dvdfileUtf16,
+ dvdemptyuuidUtf16,
+ &dvdImage);
# endif
+ }
+ if (dvdImage) {
+ rc = dvdImage->vtbl->imedium.GetId((IMedium *)dvdImage,
+ &dvduuid);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("can't get the uuid of the file to
"
+ "be attached to cdrom: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ rc = dvdDrive->vtbl->MountImage(dvdDrive, dvduuid);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not attach the file to cdrom: %s,
rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ DEBUGIID("CD/DVDImage UUID:", dvduuid);
}
- if (dvdImage) {
- rc = dvdImage->vtbl->imedium.GetId((IMedium
*)dvdImage, &dvduuid);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("can't get the uuid of the file
to "
- "be attached to cdrom: %s,
rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
- } else {
- rc = dvdDrive->vtbl->MountImage(dvdDrive,
dvduuid);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not attach the file to
cdrom: %s, rc=%08x"),
- def->disks[i]->src,
(unsigned)rc);
- } else {
- DEBUGIID("CD/DVDImage UUID:",
dvduuid);
- }
- }
-
- VBOX_MEDIUM_RELEASE(dvdImage);
- }
- vboxIIDUnalloc(dvduuid);
- VBOX_UTF16_FREE(dvdfileUtf16);
- VBOX_RELEASE(dvdDrive);
}
- } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK)
{
+
+ VBOX_MEDIUM_RELEASE(dvdImage);
}
- } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE
&&
- def->disks[i]->src != NULL) {
- IHardDisk *hardDisk = NULL;
- PRUnichar *hddfileUtf16 = NULL;
- vboxIID *hdduuid = NULL;
- PRUnichar *hddEmpty = NULL;
- /* Current Limitation: Harddisk can't be connected to
- * Secondary Master as Secondary Master is always used
- * for CD/DVD Drive, so don't connect the harddisk if it
- * is requested to be connected to Secondary master
- */
+ vboxIIDUnalloc(dvduuid);
+ VBOX_UTF16_FREE(dvdfileUtf16);
+ VBOX_RELEASE(dvdDrive);
+ }
+ } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
+ }
+ } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE &&
+ def->disks[i]->src != NULL) {
+ IHardDisk *hardDisk = NULL;
+ PRUnichar *hddfileUtf16 = NULL;
+ vboxIID *hdduuid = NULL;
+ PRUnichar *hddEmpty = NULL;
+ /* Current Limitation: Harddisk can't be connected to
+ * Secondary Master as Secondary Master is always used
+ * for CD/DVD Drive, so don't connect the harddisk if it
+ * is requested to be connected to Secondary master
+ */
- VBOX_UTF8_TO_UTF16(def->disks[i]->src, &hddfileUtf16);
- VBOX_UTF8_TO_UTF16("", &hddEmpty);
+ VBOX_UTF8_TO_UTF16(def->disks[i]->src, &hddfileUtf16);
+ VBOX_UTF8_TO_UTF16("", &hddEmpty);
- data->vboxObj->vtbl->FindHardDisk(data->vboxObj,
hddfileUtf16, &hardDisk);
+ data->vboxObj->vtbl->FindHardDisk(data->vboxObj,
hddfileUtf16,
+ &hardDisk);
- if (!hardDisk) {
+ if (!hardDisk) {
# if VBOX_API_VERSION == 2002
- data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
- hddfileUtf16,
- AccessMode_ReadWrite,
- &hardDisk);
+ data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
+ hddfileUtf16,
+ AccessMode_ReadWrite,
+ &hardDisk);
# else
- data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
- hddfileUtf16,
- AccessMode_ReadWrite,
- 0,
- hddEmpty,
- 0,
- hddEmpty,
- &hardDisk);
+ data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
+ hddfileUtf16,
+ AccessMode_ReadWrite,
+ 0,
+ hddEmpty,
+ 0,
+ hddEmpty,
+ &hardDisk);
# endif
- }
+ }
- if (hardDisk) {
- rc = hardDisk->vtbl->imedium.GetId((IMedium *)hardDisk,
&hdduuid);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("can't get the uuid of the file to
be "
- "attached as harddisk: %s,
rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
+ if (hardDisk) {
+ rc = hardDisk->vtbl->imedium.GetId((IMedium *)hardDisk,
+ &hdduuid);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("can't get the uuid of the file to be
"
+ "attached as harddisk: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ if (def->disks[i]->readonly) {
+ hardDisk->vtbl->SetType(hardDisk,
+ HardDiskType_Immutable);
+ DEBUG0("setting harddisk to readonly");
+ } else if (!def->disks[i]->readonly) {
+ hardDisk->vtbl->SetType(hardDisk,
+ HardDiskType_Normal);
+ DEBUG0("setting harddisk type to normal");
+ }
+ if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_IDE) {
+ if (STREQ(def->disks[i]->dst, "hdc")) {
+ DEBUG0("Not connecting harddisk to hdc as hdc"
+ " is taken by CD/DVD Drive");
} else {
- if (def->disks[i]->readonly) {
- hardDisk->vtbl->SetType(hardDisk,
HardDiskType_Immutable);
- DEBUG0("setting harddisk to readonly");
- } else if (!def->disks[i]->readonly) {
- hardDisk->vtbl->SetType(hardDisk,
HardDiskType_Normal);
- DEBUG0("setting harddisk type to normal");
+ PRInt32 channel = 0;
+ PRInt32 device = 0;
+ PRUnichar *hddcnameUtf16 = NULL;
+
+ char *hddcname = strdup("IDE");
+ VBOX_UTF8_TO_UTF16(hddcname, &hddcnameUtf16);
+ VIR_FREE(hddcname);
+
+ if (STREQ(def->disks[i]->dst, "hda")) {
+ channel = 0;
+ device = 0;
+ } else if (STREQ(def->disks[i]->dst,
"hdb")) {
+ channel = 0;
+ device = 1;
+ } else if (STREQ(def->disks[i]->dst,
"hdd")) {
+ channel = 1;
+ device = 1;
}
- if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_IDE)
{
- if (STREQ(def->disks[i]->dst, "hdc"))
{
- DEBUG0("Not connecting harddisk to hdc as
hdc"
- " is taken by CD/DVD Drive");
- } else {
- PRInt32 channel = 0;
- PRInt32 device = 0;
- PRUnichar *hddcnameUtf16 = NULL;
-
- char *hddcname = strdup("IDE");
- VBOX_UTF8_TO_UTF16(hddcname,
&hddcnameUtf16);
- VIR_FREE(hddcname);
-
- if (STREQ(def->disks[i]->dst,
"hda")) {
- channel = 0;
- device = 0;
- } else if (STREQ(def->disks[i]->dst,
"hdb")) {
- channel = 0;
- device = 1;
- } else if (STREQ(def->disks[i]->dst,
"hdd")) {
- channel = 1;
- device = 1;
- }
- rc =
machine->vtbl->AttachHardDisk(machine,
- hdduuid,
-
hddcnameUtf16,
- channel,
- device);
- VBOX_UTF16_FREE(hddcnameUtf16);
+ rc = machine->vtbl->AttachHardDisk(machine,
+ hdduuid,
+ hddcnameUtf16,
+ channel,
+ device);
+ VBOX_UTF16_FREE(hddcnameUtf16);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not attach the file
as "
- "harddisk: %s,
rc=%08x"),
- def->disks[i]->src,
(unsigned)rc);
- } else {
- DEBUGIID("Attached HDD with UUID",
hdduuid);
- }
- }
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not attach the file as
"
+ "harddisk: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ DEBUGIID("Attached HDD with UUID",
hdduuid);
}
}
- VBOX_MEDIUM_RELEASE(hardDisk);
}
- vboxIIDUnalloc(hdduuid);
- VBOX_UTF16_FREE(hddEmpty);
- VBOX_UTF16_FREE(hddfileUtf16);
- } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK)
{
}
- } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
{
- if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE
&&
- def->disks[i]->src != NULL) {
- IFloppyDrive *floppyDrive;
- machine->vtbl->GetFloppyDrive(machine, &floppyDrive);
- if (floppyDrive) {
- rc = floppyDrive->vtbl->SetEnabled(floppyDrive, 1);
- if (NS_SUCCEEDED(rc)) {
- IFloppyImage *floppyImage = NULL;
- PRUnichar *fdfileUtf16 = NULL;
- vboxIID *fduuid = NULL;
+ VBOX_MEDIUM_RELEASE(hardDisk);
+ }
+ vboxIIDUnalloc(hdduuid);
+ VBOX_UTF16_FREE(hddEmpty);
+ VBOX_UTF16_FREE(hddfileUtf16);
+ } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
+ }
+ } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE &&
+ def->disks[i]->src != NULL) {
+ IFloppyDrive *floppyDrive;
+ machine->vtbl->GetFloppyDrive(machine, &floppyDrive);
+ if (floppyDrive) {
+ rc = floppyDrive->vtbl->SetEnabled(floppyDrive, 1);
+ if (NS_SUCCEEDED(rc)) {
+ IFloppyImage *floppyImage = NULL;
+ PRUnichar *fdfileUtf16 = NULL;
+ vboxIID *fduuid = NULL;
# if VBOX_API_VERSION == 2002
- nsID fdemptyuuid;
+ nsID fdemptyuuid;
- memset(&fdemptyuuid, 0, sizeof(fdemptyuuid));
+ memset(&fdemptyuuid, 0, sizeof(fdemptyuuid));
# else
- PRUnichar *fdemptyuuidUtf16 = NULL;
+ PRUnichar *fdemptyuuidUtf16 = NULL;
# endif
- VBOX_UTF8_TO_UTF16(def->disks[i]->src,
&fdfileUtf16);
- rc =
data->vboxObj->vtbl->FindFloppyImage(data->vboxObj,
- fdfileUtf16,
-
&floppyImage);
+ VBOX_UTF8_TO_UTF16(def->disks[i]->src, &fdfileUtf16);
+ rc =
data->vboxObj->vtbl->FindFloppyImage(data->vboxObj,
+ fdfileUtf16,
+ &floppyImage);
- if (!floppyImage) {
-
data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj,
- fdfileUtf16,
+ if (!floppyImage) {
+
data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj,
+ fdfileUtf16,
# if VBOX_API_VERSION == 2002
-
&fdemptyuuid,
+ &fdemptyuuid,
# else
-
fdemptyuuidUtf16,
+ fdemptyuuidUtf16,
# endif
-
&floppyImage);
- }
+ &floppyImage);
+ }
- if (floppyImage) {
- rc = floppyImage->vtbl->imedium.GetId((IMedium
*)floppyImage, &fduuid);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("can't get the uuid of the
file to "
- "be attached to floppy drive:
%s, rc=%08x"),
- def->disks[i]->src,
(unsigned)rc);
- } else {
- rc =
floppyDrive->vtbl->MountImage(floppyDrive, fduuid);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not attach the file
to "
- "floppy drive: %s,
rc=%08x"),
- def->disks[i]->src,
(unsigned)rc);
- } else {
- DEBUGIID("floppyImage UUID",
fduuid);
- }
- }
- VBOX_MEDIUM_RELEASE(floppyImage);
+ if (floppyImage) {
+ rc = floppyImage->vtbl->imedium.GetId((IMedium
*)floppyImage,
+ &fduuid);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("can't get the uuid of the file to
"
+ "be attached to floppy drive: %s,
rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ rc = floppyDrive->vtbl->MountImage(floppyDrive,
+ fduuid);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not attach the file to
"
+ "floppy drive: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ DEBUGIID("floppyImage UUID", fduuid);
}
- vboxIIDUnalloc(fduuid);
- VBOX_UTF16_FREE(fdfileUtf16);
}
- VBOX_RELEASE(floppyDrive);
+ VBOX_MEDIUM_RELEASE(floppyImage);
}
- } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK)
{
+ vboxIIDUnalloc(fduuid);
+ VBOX_UTF16_FREE(fdfileUtf16);
}
+ VBOX_RELEASE(floppyDrive);
}
+ } else if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
}
}
-
- } /* Finished:Block to attach the CDROM/DVD Drive and HardDisks to the VM */
-
+ }
#else /* VBOX_API_VERSION >= 3001 */
+ PRUint32 maxPortPerInst[StorageBus_Floppy + 1] = {};
+ PRUint32 maxSlotPerPort[StorageBus_Floppy + 1] = {};
+ PRUnichar *storageCtlName = NULL;
+ bool error = false;
+
+ /* get the max port/slots/etc for the given storage bus */
+ error = !vboxGetMaxPortSlotValues(data->vboxObj, maxPortPerInst,
+ maxSlotPerPort);
+
+ /* add a storage controller for the mediums to be attached */
+ /* this needs to change when multiple controller are supported for
+ * ver > 3.1 */
{
- PRUint32 maxPortPerInst[StorageBus_Floppy + 1] = {};
- PRUint32 maxSlotPerPort[StorageBus_Floppy + 1] = {};
- PRUnichar *storageCtlName = NULL;
- bool error = false;
- int i = 0;
-
- /* get the max port/slots/etc for the given storage bus */
- error = !vboxGetMaxPortSlotValues(data->vboxObj, maxPortPerInst,
maxSlotPerPort);
-
- /* add a storage controller for the mediums to be attached */
- /* this needs to change when multiple controller are supported for ver > 3.1
*/
- {
- IStorageController *storageCtl = NULL;
- PRUnichar *sName = NULL;
-
- VBOX_UTF8_TO_UTF16("IDE Controller", &sName);
- machine->vtbl->AddStorageController(machine,
- sName,
- StorageBus_IDE,
- &storageCtl);
- VBOX_UTF16_FREE(sName);
- VBOX_RELEASE(storageCtl);
-
- VBOX_UTF8_TO_UTF16("SATA Controller", &sName);
- machine->vtbl->AddStorageController(machine,
- sName,
- StorageBus_SATA,
- &storageCtl);
- VBOX_UTF16_FREE(sName);
- VBOX_RELEASE(storageCtl);
-
- VBOX_UTF8_TO_UTF16("SCSI Controller", &sName);
- machine->vtbl->AddStorageController(machine,
- sName,
- StorageBus_SCSI,
- &storageCtl);
- VBOX_UTF16_FREE(sName);
- VBOX_RELEASE(storageCtl);
-
- VBOX_UTF8_TO_UTF16("Floppy Controller", &sName);
- machine->vtbl->AddStorageController(machine,
- sName,
- StorageBus_Floppy,
- &storageCtl);
- VBOX_UTF16_FREE(sName);
- VBOX_RELEASE(storageCtl);
- }
-
- /* Started:Block to attach the CDROM/DVD Drive and HardDisks to the VM */
- for (i = 0; i < def->ndisks && !error; i++) {
- DEBUG("disk(%d) type: %d", i, def->disks[i]->type);
- DEBUG("disk(%d) device: %d", i, def->disks[i]->device);
- DEBUG("disk(%d) bus: %d", i, def->disks[i]->bus);
- DEBUG("disk(%d) src: %s", i, def->disks[i]->src);
- DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst);
- DEBUG("disk(%d) driverName: %s", i,
def->disks[i]->driverName);
- DEBUG("disk(%d) driverType: %s", i,
def->disks[i]->driverType);
- DEBUG("disk(%d) cachemode: %d", i,
def->disks[i]->cachemode);
- DEBUG("disk(%d) readonly: %s", i, def->disks[i]->readonly ?
"True" : "False");
- DEBUG("disk(%d) shared: %s", i, def->disks[i]->shared ?
"True" : "False");
+ IStorageController *storageCtl = NULL;
+ PRUnichar *sName = NULL;
+
+ VBOX_UTF8_TO_UTF16("IDE Controller", &sName);
+ machine->vtbl->AddStorageController(machine,
+ sName,
+ StorageBus_IDE,
+ &storageCtl);
+ VBOX_UTF16_FREE(sName);
+ VBOX_RELEASE(storageCtl);
+
+ VBOX_UTF8_TO_UTF16("SATA Controller", &sName);
+ machine->vtbl->AddStorageController(machine,
+ sName,
+ StorageBus_SATA,
+ &storageCtl);
+ VBOX_UTF16_FREE(sName);
+ VBOX_RELEASE(storageCtl);
+
+ VBOX_UTF8_TO_UTF16("SCSI Controller", &sName);
+ machine->vtbl->AddStorageController(machine,
+ sName,
+ StorageBus_SCSI,
+ &storageCtl);
+ VBOX_UTF16_FREE(sName);
+ VBOX_RELEASE(storageCtl);
+
+ VBOX_UTF8_TO_UTF16("Floppy Controller", &sName);
+ machine->vtbl->AddStorageController(machine,
+ sName,
+ StorageBus_Floppy,
+ &storageCtl);
+ VBOX_UTF16_FREE(sName);
+ VBOX_RELEASE(storageCtl);
+ }
+
+ for (i = 0; i < def->ndisks && !error; i++) {
+ DEBUG("disk(%d) type: %d", i, def->disks[i]->type);
+ DEBUG("disk(%d) device: %d", i, def->disks[i]->device);
+ DEBUG("disk(%d) bus: %d", i, def->disks[i]->bus);
+ DEBUG("disk(%d) src: %s", i, def->disks[i]->src);
+ DEBUG("disk(%d) dst: %s", i, def->disks[i]->dst);
+ DEBUG("disk(%d) driverName: %s", i, def->disks[i]->driverName);
+ DEBUG("disk(%d) driverType: %s", i, def->disks[i]->driverType);
+ DEBUG("disk(%d) cachemode: %d", i, def->disks[i]->cachemode);
+ DEBUG("disk(%d) readonly: %s", i, (def->disks[i]->readonly
+ ? "True" : "False"));
+ DEBUG("disk(%d) shared: %s", i, (def->disks[i]->shared
+ ? "True" : "False"));
+
+ if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE &&
+ def->disks[i]->src != NULL) {
+ IMedium *medium = NULL;
+ PRUnichar *mediumUUID = NULL;
+ PRUnichar *mediumFileUtf16 = NULL;
+ PRUint32 storageBus = StorageBus_Null;
+ PRUint32 deviceType = DeviceType_Null;
+ PRInt32 deviceInst = 0;
+ PRInt32 devicePort = 0;
+ PRInt32 deviceSlot = 0;
+
+ VBOX_UTF8_TO_UTF16(def->disks[i]->src, &mediumFileUtf16);
+
+ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ deviceType = DeviceType_HardDisk;
+ data->vboxObj->vtbl->FindHardDisk(data->vboxObj,
+ mediumFileUtf16, &medium);
+ } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
+ deviceType = DeviceType_DVD;
+ data->vboxObj->vtbl->FindDVDImage(data->vboxObj,
+ mediumFileUtf16, &medium);
+ } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ deviceType = DeviceType_Floppy;
+ data->vboxObj->vtbl->FindFloppyImage(data->vboxObj,
+ mediumFileUtf16, &medium);
+ } else {
+ VBOX_UTF16_FREE(mediumFileUtf16);
+ continue;
+ }
- if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE &&
- def->disks[i]->src != NULL) {
- IMedium *medium = NULL;
- PRUnichar *mediumUUID = NULL;
- PRUnichar *mediumFileUtf16 = NULL;
- PRUint32 storageBus = StorageBus_Null;
- PRUint32 deviceType = DeviceType_Null;
- PRInt32 deviceInst = 0;
- PRInt32 devicePort = 0;
- PRInt32 deviceSlot = 0;
+ if (!medium) {
+ PRUnichar *mediumEmpty = NULL;
- VBOX_UTF8_TO_UTF16(def->disks[i]->src, &mediumFileUtf16);
+ VBOX_UTF8_TO_UTF16("", &mediumEmpty);
if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- deviceType = DeviceType_HardDisk;
- data->vboxObj->vtbl->FindHardDisk(data->vboxObj,
mediumFileUtf16, &medium);
- } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
{
- deviceType = DeviceType_DVD;
- data->vboxObj->vtbl->FindDVDImage(data->vboxObj,
mediumFileUtf16, &medium);
- } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
{
- deviceType = DeviceType_Floppy;
- data->vboxObj->vtbl->FindFloppyImage(data->vboxObj,
mediumFileUtf16, &medium);
- } else {
- VBOX_UTF16_FREE(mediumFileUtf16);
- continue;
+ data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
+ mediumFileUtf16,
+ AccessMode_ReadWrite,
+ false,
+ mediumEmpty,
+ false,
+ mediumEmpty,
+ &medium);
+ } else if (def->disks[i]->device ==
+ VIR_DOMAIN_DISK_DEVICE_CDROM) {
+ data->vboxObj->vtbl->OpenDVDImage(data->vboxObj,
+ mediumFileUtf16,
+ mediumEmpty,
+ &medium);
+ } else if (def->disks[i]->device ==
+ VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj,
+ mediumFileUtf16,
+ mediumEmpty,
+ &medium);
}
- if (!medium) {
- PRUnichar *mediumEmpty = NULL;
-
- VBOX_UTF8_TO_UTF16("", &mediumEmpty);
-
- if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- data->vboxObj->vtbl->OpenHardDisk(data->vboxObj,
- mediumFileUtf16,
- AccessMode_ReadWrite,
- false,
- mediumEmpty,
- false,
- mediumEmpty,
- &medium);
- } else if (def->disks[i]->device ==
VIR_DOMAIN_DISK_DEVICE_CDROM) {
- data->vboxObj->vtbl->OpenDVDImage(data->vboxObj,
- mediumFileUtf16,
- mediumEmpty,
- &medium);
- } else if (def->disks[i]->device ==
VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
- data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj,
- mediumFileUtf16,
- mediumEmpty,
- &medium);
- }
-
- VBOX_UTF16_FREE(mediumEmpty);
- }
-
- if (!medium) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to attach the following disk/dvd/floppy
"
- "to the machine: %s, rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
- VBOX_UTF16_FREE(mediumFileUtf16);
- continue;
- }
-
- rc = medium->vtbl->GetId(medium, &mediumUUID);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("can't get the uuid of the file to be attached
"
- "as harddisk/dvd/floppy: %s, rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
- VBOX_RELEASE(medium);
- VBOX_UTF16_FREE(mediumFileUtf16);
- continue;
- }
+ VBOX_UTF16_FREE(mediumEmpty);
+ }
- if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- if (def->disks[i]->readonly) {
- medium->vtbl->SetType(medium, MediumType_Immutable);
- DEBUG0("setting harddisk to immutable");
- } else if (!def->disks[i]->readonly) {
- medium->vtbl->SetType(medium, MediumType_Normal);
- DEBUG0("setting harddisk type to normal");
- }
- }
+ if (!medium) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to attach the following disk/dvd/floppy "
+ "to the machine: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ VBOX_UTF16_FREE(mediumFileUtf16);
+ continue;
+ }
- if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_IDE) {
- VBOX_UTF8_TO_UTF16("IDE Controller", &storageCtlName);
- storageBus = StorageBus_IDE;
- } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_SATA) {
- VBOX_UTF8_TO_UTF16("SATA Controller",
&storageCtlName);
- storageBus = StorageBus_SATA;
- } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
- VBOX_UTF8_TO_UTF16("SCSI Controller",
&storageCtlName);
- storageBus = StorageBus_SCSI;
- } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_FDC) {
- VBOX_UTF8_TO_UTF16("Floppy Controller",
&storageCtlName);
- storageBus = StorageBus_Floppy;
- }
+ rc = medium->vtbl->GetId(medium, &mediumUUID);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("can't get the uuid of the file to be attached
"
+ "as harddisk/dvd/floppy: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ VBOX_RELEASE(medium);
+ VBOX_UTF16_FREE(mediumFileUtf16);
+ continue;
+ }
- /* get the device details i.e instance, port and slot */
- if (!vboxGetDeviceDetails(def->disks[i]->dst,
- maxPortPerInst,
- maxSlotPerPort,
- storageBus,
- &deviceInst,
- &devicePort,
- &deviceSlot)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("can't get the port/slot number of
harddisk/"
- "dvd/floppy to be attached: %s, rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
- VBOX_RELEASE(medium);
- VBOX_UTF16_FREE(mediumUUID);
- VBOX_UTF16_FREE(mediumFileUtf16);
- continue;
+ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (def->disks[i]->readonly) {
+ medium->vtbl->SetType(medium, MediumType_Immutable);
+ DEBUG0("setting harddisk to immutable");
+ } else if (!def->disks[i]->readonly) {
+ medium->vtbl->SetType(medium, MediumType_Normal);
+ DEBUG0("setting harddisk type to normal");
}
+ }
- /* attach the harddisk/dvd/Floppy to the storage controller */
- rc = machine->vtbl->AttachDevice(machine,
- storageCtlName,
- devicePort,
- deviceSlot,
- deviceType,
- mediumUUID);
-
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not attach the file as harddisk/"
- "dvd/floppy: %s, rc=%08x"),
- def->disks[i]->src, (unsigned)rc);
- } else {
- DEBUGIID("Attached HDD/DVD/Floppy with UUID", mediumUUID);
- }
+ if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_IDE) {
+ VBOX_UTF8_TO_UTF16("IDE Controller", &storageCtlName);
+ storageBus = StorageBus_IDE;
+ } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_SATA) {
+ VBOX_UTF8_TO_UTF16("SATA Controller", &storageCtlName);
+ storageBus = StorageBus_SATA;
+ } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+ VBOX_UTF8_TO_UTF16("SCSI Controller", &storageCtlName);
+ storageBus = StorageBus_SCSI;
+ } else if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_FDC) {
+ VBOX_UTF8_TO_UTF16("Floppy Controller", &storageCtlName);
+ storageBus = StorageBus_Floppy;
+ }
+ /* get the device details i.e instance, port and slot */
+ if (!vboxGetDeviceDetails(def->disks[i]->dst,
+ maxPortPerInst,
+ maxSlotPerPort,
+ storageBus,
+ &deviceInst,
+ &devicePort,
+ &deviceSlot)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("can't get the port/slot number of harddisk/"
+ "dvd/floppy to be attached: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
VBOX_RELEASE(medium);
VBOX_UTF16_FREE(mediumUUID);
VBOX_UTF16_FREE(mediumFileUtf16);
- VBOX_UTF16_FREE(storageCtlName);
+ continue;
+ }
+
+ /* attach the harddisk/dvd/Floppy to the storage controller */
+ rc = machine->vtbl->AttachDevice(machine,
+ storageCtlName,
+ devicePort,
+ deviceSlot,
+ deviceType,
+ mediumUUID);
+
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not attach the file as harddisk/"
+ "dvd/floppy: %s, rc=%08x"),
+ def->disks[i]->src, (unsigned)rc);
+ } else {
+ DEBUGIID("Attached HDD/DVD/Floppy with UUID", mediumUUID);
}
+
+ VBOX_RELEASE(medium);
+ VBOX_UTF16_FREE(mediumUUID);
+ VBOX_UTF16_FREE(mediumFileUtf16);
+ VBOX_UTF16_FREE(storageCtlName);
}
}
- /* Finished:Block to attach the CDROM/DVD Drive and HardDisks to the VM */
#endif /* VBOX_API_VERSION >= 3001 */
+}
- { /* Started:Block to attach the Sound Controller to the VM */
- /* Check if def->nsounds is one as VirtualBox currently supports
- * only one sound card
- */
- if (def->nsounds == 1) {
- IAudioAdapter *audioAdapter = NULL;
+static void
+vboxAttachSound(virDomainDefPtr def, IMachine *machine)
+{
+ nsresult rc;
- machine->vtbl->GetAudioAdapter(machine, &audioAdapter);
- if (audioAdapter) {
- rc = audioAdapter->vtbl->SetEnabled(audioAdapter, 1);
- if (NS_SUCCEEDED(rc)) {
- if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_SB16) {
- audioAdapter->vtbl->SetAudioController(audioAdapter,
AudioControllerType_SB16);
- } else if (def->sounds[0]->model ==
VIR_DOMAIN_SOUND_MODEL_AC97) {
- audioAdapter->vtbl->SetAudioController(audioAdapter,
AudioControllerType_AC97);
- }
+ /* Check if def->nsounds is one as VirtualBox currently supports
+ * only one sound card
+ */
+ if (def->nsounds == 1) {
+ IAudioAdapter *audioAdapter = NULL;
+
+ machine->vtbl->GetAudioAdapter(machine, &audioAdapter);
+ if (audioAdapter) {
+ rc = audioAdapter->vtbl->SetEnabled(audioAdapter, 1);
+ if (NS_SUCCEEDED(rc)) {
+ if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_SB16) {
+ audioAdapter->vtbl->SetAudioController(audioAdapter,
+ AudioControllerType_SB16);
+ } else if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_AC97) {
+ audioAdapter->vtbl->SetAudioController(audioAdapter,
+ AudioControllerType_AC97);
}
- VBOX_RELEASE(audioAdapter);
}
+ VBOX_RELEASE(audioAdapter);
}
- } /* Finished:Block to attach the Sound Controller to the VM */
-
- { /* Started:Block to attach the Network Card to the VM */
- ISystemProperties *systemProperties = NULL;
- PRUint32 networkAdapterCount = 0;
- int i = 0;
-
- data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
- if (systemProperties) {
- systemProperties->vtbl->GetNetworkAdapterCount(systemProperties,
&networkAdapterCount);
- VBOX_RELEASE(systemProperties);
- systemProperties = NULL;
+ }
+}
+
+static void
+vboxAttachNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ ISystemProperties *systemProperties = NULL;
+ PRUint32 networkAdapterCount = 0;
+ int i = 0;
+
+ data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
+ if (systemProperties) {
+ systemProperties->vtbl->GetNetworkAdapterCount(systemProperties,
+ &networkAdapterCount);
+ VBOX_RELEASE(systemProperties);
+ systemProperties = NULL;
+ }
+
+ DEBUG("Number of Network Cards to be connected: %d", def->nnets);
+ DEBUG("Number of Network Cards available: %d", networkAdapterCount);
+
+ for (i = 0; (i < def->nnets) && (i < networkAdapterCount); i++) {
+ INetworkAdapter *adapter = NULL;
+ PRUint32 adapterType = NetworkAdapterType_Null;
+ char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
+ char macaddrvbox[VIR_MAC_STRING_BUFLEN - 5] = {0};
+
+ virFormatMacAddr(def->nets[i]->mac, macaddr);
+ snprintf(macaddrvbox, VIR_MAC_STRING_BUFLEN - 5,
+ "%02X%02X%02X%02X%02X%02X",
+ def->nets[i]->mac[0],
+ def->nets[i]->mac[1],
+ def->nets[i]->mac[2],
+ def->nets[i]->mac[3],
+ def->nets[i]->mac[4],
+ def->nets[i]->mac[5]);
+ macaddrvbox[VIR_MAC_STRING_BUFLEN - 6] = '\0';
+
+ DEBUG("NIC(%d): Type: %d", i, def->nets[i]->type);
+ DEBUG("NIC(%d): Model: %s", i, def->nets[i]->model);
+ DEBUG("NIC(%d): Mac: %s", i, macaddr);
+ DEBUG("NIC(%d): ifname: %s", i, def->nets[i]->ifname);
+ if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ DEBUG("NIC(%d): name: %s", i,
def->nets[i]->data.network.name);
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_INTERNAL) {
+ DEBUG("NIC(%d): name: %s", i,
def->nets[i]->data.internal.name);
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_USER) {
+ DEBUG("NIC(%d): NAT.", i);
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ DEBUG("NIC(%d): brname: %s", i,
def->nets[i]->data.bridge.brname);
+ DEBUG("NIC(%d): script: %s", i,
def->nets[i]->data.bridge.script);
+ DEBUG("NIC(%d): ipaddr: %s", i,
def->nets[i]->data.bridge.ipaddr);
}
- DEBUG("Number of Network Cards to be connected: %d", def->nnets);
- DEBUG("Number of Network Cards available: %d", networkAdapterCount);
-
- for (i = 0; (i < def->nnets) && (i < networkAdapterCount); i++)
{
- INetworkAdapter *adapter = NULL;
- PRUint32 adapterType = NetworkAdapterType_Null;
- char macaddr[VIR_MAC_STRING_BUFLEN] = {0};
- char macaddrvbox[VIR_MAC_STRING_BUFLEN - 5] = {0};
-
- virFormatMacAddr(def->nets[i]->mac, macaddr);
- snprintf(macaddrvbox, VIR_MAC_STRING_BUFLEN - 5,
- "%02X%02X%02X%02X%02X%02X",
- def->nets[i]->mac[0],
- def->nets[i]->mac[1],
- def->nets[i]->mac[2],
- def->nets[i]->mac[3],
- def->nets[i]->mac[4],
- def->nets[i]->mac[5]);
- macaddrvbox[VIR_MAC_STRING_BUFLEN - 6] = '\0';
-
- DEBUG("NIC(%d): Type: %d", i, def->nets[i]->type);
- DEBUG("NIC(%d): Model: %s", i, def->nets[i]->model);
- DEBUG("NIC(%d): Mac: %s", i, macaddr);
- DEBUG("NIC(%d): ifname: %s", i, def->nets[i]->ifname);
- if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- DEBUG("NIC(%d): name: %s", i,
def->nets[i]->data.network.name);
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_INTERNAL) {
- DEBUG("NIC(%d): name: %s", i,
def->nets[i]->data.internal.name);
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_USER) {
- DEBUG("NIC(%d): NAT.", i);
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- DEBUG("NIC(%d): brname: %s", i,
def->nets[i]->data.bridge.brname);
- DEBUG("NIC(%d): script: %s", i,
def->nets[i]->data.bridge.script);
- DEBUG("NIC(%d): ipaddr: %s", i,
def->nets[i]->data.bridge.ipaddr);
- }
+ machine->vtbl->GetNetworkAdapter(machine, i, &adapter);
+ if (adapter) {
+ PRUnichar *MACAddress = NULL;
- machine->vtbl->GetNetworkAdapter(machine, i, &adapter);
- if (adapter) {
- PRUnichar *MACAddress = NULL;
-
- adapter->vtbl->SetEnabled(adapter, 1);
-
- if (def->nets[i]->model) {
- if (STRCASEEQ(def->nets[i]->model , "Am79C970A")) {
- adapterType = NetworkAdapterType_Am79C970A;
- } else if (STRCASEEQ(def->nets[i]->model ,
"Am79C973")) {
- adapterType = NetworkAdapterType_Am79C973;
- } else if (STRCASEEQ(def->nets[i]->model ,
"82540EM")) {
- adapterType = NetworkAdapterType_I82540EM;
- } else if (STRCASEEQ(def->nets[i]->model ,
"82545EM")) {
- adapterType = NetworkAdapterType_I82545EM;
- } else if (STRCASEEQ(def->nets[i]->model ,
"82543GC")) {
- adapterType = NetworkAdapterType_I82543GC;
+ adapter->vtbl->SetEnabled(adapter, 1);
+
+ if (def->nets[i]->model) {
+ if (STRCASEEQ(def->nets[i]->model , "Am79C970A")) {
+ adapterType = NetworkAdapterType_Am79C970A;
+ } else if (STRCASEEQ(def->nets[i]->model , "Am79C973"))
{
+ adapterType = NetworkAdapterType_Am79C973;
+ } else if (STRCASEEQ(def->nets[i]->model , "82540EM")) {
+ adapterType = NetworkAdapterType_I82540EM;
+ } else if (STRCASEEQ(def->nets[i]->model , "82545EM")) {
+ adapterType = NetworkAdapterType_I82545EM;
+ } else if (STRCASEEQ(def->nets[i]->model , "82543GC")) {
+ adapterType = NetworkAdapterType_I82543GC;
#if VBOX_API_VERSION >= 3001
- } else if (STRCASEEQ(def->nets[i]->model , "virtio"))
{
- adapterType = NetworkAdapterType_Virtio;
+ } else if (STRCASEEQ(def->nets[i]->model , "virtio")) {
+ adapterType = NetworkAdapterType_Virtio;
#endif /* VBOX_API_VERSION >= 3001 */
- }
- } else {
- adapterType = NetworkAdapterType_Am79C973;
}
+ } else {
+ adapterType = NetworkAdapterType_Am79C973;
+ }
- adapter->vtbl->SetAdapterType(adapter, adapterType);
+ adapter->vtbl->SetAdapterType(adapter, adapterType);
- if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- PRUnichar *hostInterface = NULL;
- /* Bridged Network */
+ if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ PRUnichar *hostInterface = NULL;
+ /* Bridged Network */
- adapter->vtbl->AttachToBridgedInterface(adapter);
+ adapter->vtbl->AttachToBridgedInterface(adapter);
- if (def->nets[i]->data.bridge.brname) {
- VBOX_UTF8_TO_UTF16(def->nets[i]->data.bridge.brname,
&hostInterface);
- adapter->vtbl->SetHostInterface(adapter, hostInterface);
- VBOX_UTF16_FREE(hostInterface);
- }
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_INTERNAL) {
- PRUnichar *internalNetwork = NULL;
- /* Internal Network */
-
- adapter->vtbl->AttachToInternalNetwork(adapter);
+ if (def->nets[i]->data.bridge.brname) {
+ VBOX_UTF8_TO_UTF16(def->nets[i]->data.bridge.brname,
+ &hostInterface);
+ adapter->vtbl->SetHostInterface(adapter, hostInterface);
+ VBOX_UTF16_FREE(hostInterface);
+ }
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_INTERNAL) {
+ PRUnichar *internalNetwork = NULL;
+ /* Internal Network */
- if (def->nets[i]->data.internal.name) {
- VBOX_UTF8_TO_UTF16(def->nets[i]->data.internal.name,
&internalNetwork);
- adapter->vtbl->SetInternalNetwork(adapter,
internalNetwork);
- VBOX_UTF16_FREE(internalNetwork);
- }
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- PRUnichar *hostInterface = NULL;
- /* Host Only Networking (currently only vboxnet0 available
- * on *nix and mac, on windows you can create and configure
- * as many as you want)
- */
- adapter->vtbl->AttachToHostOnlyInterface(adapter);
+ adapter->vtbl->AttachToInternalNetwork(adapter);
- if (def->nets[i]->data.network.name) {
- VBOX_UTF8_TO_UTF16(def->nets[i]->data.network.name,
&hostInterface);
- adapter->vtbl->SetHostInterface(adapter, hostInterface);
- VBOX_UTF16_FREE(hostInterface);
- }
- } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_USER) {
- /* NAT */
- adapter->vtbl->AttachToNAT(adapter);
- } else {
- /* else always default to NAT if we don't understand
- * what option is been passed to us
- */
- adapter->vtbl->AttachToNAT(adapter);
+ if (def->nets[i]->data.internal.name) {
+ VBOX_UTF8_TO_UTF16(def->nets[i]->data.internal.name,
+ &internalNetwork);
+ adapter->vtbl->SetInternalNetwork(adapter, internalNetwork);
+ VBOX_UTF16_FREE(internalNetwork);
}
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ PRUnichar *hostInterface = NULL;
+ /* Host Only Networking (currently only vboxnet0 available
+ * on *nix and mac, on windows you can create and configure
+ * as many as you want)
+ */
+ adapter->vtbl->AttachToHostOnlyInterface(adapter);
- VBOX_UTF8_TO_UTF16(macaddrvbox, &MACAddress);
- adapter->vtbl->SetMACAddress(adapter, MACAddress);
- VBOX_UTF16_FREE(MACAddress);
+ if (def->nets[i]->data.network.name) {
+ VBOX_UTF8_TO_UTF16(def->nets[i]->data.network.name,
+ &hostInterface);
+ adapter->vtbl->SetHostInterface(adapter, hostInterface);
+ VBOX_UTF16_FREE(hostInterface);
+ }
+ } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_USER) {
+ /* NAT */
+ adapter->vtbl->AttachToNAT(adapter);
+ } else {
+ /* else always default to NAT if we don't understand
+ * what option is been passed to us
+ */
+ adapter->vtbl->AttachToNAT(adapter);
}
+
+ VBOX_UTF8_TO_UTF16(macaddrvbox, &MACAddress);
+ adapter->vtbl->SetMACAddress(adapter, MACAddress);
+ VBOX_UTF16_FREE(MACAddress);
}
- } /* Finished:Block to attach the Network Card to the VM */
-
- { /* Started:Block to attach the Serial Port to the VM */
- ISystemProperties *systemProperties = NULL;
- PRUint32 serialPortCount = 0;
- int i = 0;
-
- data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
- if (systemProperties) {
- systemProperties->vtbl->GetSerialPortCount(systemProperties,
&serialPortCount);
- VBOX_RELEASE(systemProperties);
- systemProperties = NULL;
- }
+ }
+}
+
+static void
+vboxAttachSerial(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ ISystemProperties *systemProperties = NULL;
+ PRUint32 serialPortCount = 0;
+ int i = 0;
- DEBUG("Number of Serial Ports to be connected: %d", def->nserials);
- DEBUG("Number of Serial Ports available: %d", serialPortCount);
- for (i = 0; (i < def->nserials) && (i < serialPortCount); i++)
{
- ISerialPort *serialPort = NULL;
+ data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
+ if (systemProperties) {
+ systemProperties->vtbl->GetSerialPortCount(systemProperties,
+ &serialPortCount);
+ VBOX_RELEASE(systemProperties);
+ systemProperties = NULL;
+ }
- DEBUG("SerialPort(%d): Type: %d", i, def->serials[i]->type);
- DEBUG("SerialPort(%d): target.port: %d", i,
def->serials[i]->target.port);
+ DEBUG("Number of Serial Ports to be connected: %d", def->nserials);
+ DEBUG("Number of Serial Ports available: %d", serialPortCount);
+ for (i = 0; (i < def->nserials) && (i < serialPortCount); i++) {
+ ISerialPort *serialPort = NULL;
- machine->vtbl->GetSerialPort(machine, i, &serialPort);
- if (serialPort) {
- PRUnichar *pathUtf16 = NULL;
+ DEBUG("SerialPort(%d): Type: %d", i, def->serials[i]->type);
+ DEBUG("SerialPort(%d): target.port: %d", i,
+ def->serials[i]->target.port);
- serialPort->vtbl->SetEnabled(serialPort, 1);
+ machine->vtbl->GetSerialPort(machine, i, &serialPort);
+ if (serialPort) {
+ PRUnichar *pathUtf16 = NULL;
- if (def->serials[i]->data.file.path) {
- VBOX_UTF8_TO_UTF16(def->serials[i]->data.file.path,
&pathUtf16);
- serialPort->vtbl->SetPath(serialPort, pathUtf16);
- }
+ serialPort->vtbl->SetEnabled(serialPort, 1);
- /* For now hard code the serial ports to COM1 and COM2,
- * COM1 (Base Addr: 0x3F8 (decimal: 1016), IRQ: 4)
- * COM2 (Base Addr: 0x2F8 (decimal: 760), IRQ: 3)
- * TODO: make this more flexible
- */
- /* TODO: to improve the libvirt XMl handling so
- * that def->serials[i]->target.port shows real port
- * and not always start at 0
- */
- if (def->serials[i]->target.port == 0) {
- serialPort->vtbl->SetIRQ(serialPort, 4);
- serialPort->vtbl->SetIOBase(serialPort, 1016);
- DEBUG(" serialPort-%d irq: %d, iobase 0x%x, path: %s",
- i, 4, 1016, def->serials[i]->data.file.path);
- } else if (def->serials[i]->target.port == 1) {
- serialPort->vtbl->SetIRQ(serialPort, 3);
- serialPort->vtbl->SetIOBase(serialPort, 760);
- DEBUG(" serialPort-%d irq: %d, iobase 0x%x, path: %s",
- i, 3, 760, def->serials[i]->data.file.path);
- }
+ if (def->serials[i]->data.file.path) {
+ VBOX_UTF8_TO_UTF16(def->serials[i]->data.file.path,
&pathUtf16);
+ serialPort->vtbl->SetPath(serialPort, pathUtf16);
+ }
+
+ /* For now hard code the serial ports to COM1 and COM2,
+ * COM1 (Base Addr: 0x3F8 (decimal: 1016), IRQ: 4)
+ * COM2 (Base Addr: 0x2F8 (decimal: 760), IRQ: 3)
+ * TODO: make this more flexible
+ */
+ /* TODO: to improve the libvirt XMl handling so
+ * that def->serials[i]->target.port shows real port
+ * and not always start at 0
+ */
+ if (def->serials[i]->target.port == 0) {
+ serialPort->vtbl->SetIRQ(serialPort, 4);
+ serialPort->vtbl->SetIOBase(serialPort, 1016);
+ DEBUG(" serialPort-%d irq: %d, iobase 0x%x, path: %s",
+ i, 4, 1016, def->serials[i]->data.file.path);
+ } else if (def->serials[i]->target.port == 1) {
+ serialPort->vtbl->SetIRQ(serialPort, 3);
+ serialPort->vtbl->SetIOBase(serialPort, 760);
+ DEBUG(" serialPort-%d irq: %d, iobase 0x%x, path: %s",
+ i, 3, 760, def->serials[i]->data.file.path);
+ }
- if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_DEV) {
- serialPort->vtbl->SetHostMode(serialPort,
PortMode_HostDevice);
- } else if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_PIPE) {
- serialPort->vtbl->SetHostMode(serialPort, PortMode_HostPipe);
+ if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_DEV) {
+ serialPort->vtbl->SetHostMode(serialPort, PortMode_HostDevice);
+ } else if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_PIPE) {
+ serialPort->vtbl->SetHostMode(serialPort, PortMode_HostPipe);
#if VBOX_API_VERSION >= 3000
- } else if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_FILE) {
- serialPort->vtbl->SetHostMode(serialPort, PortMode_RawFile);
+ } else if (def->serials[i]->type == VIR_DOMAIN_CHR_TYPE_FILE) {
+ serialPort->vtbl->SetHostMode(serialPort, PortMode_RawFile);
#endif /* VBOX_API_VERSION >= 3000 */
- } else {
- serialPort->vtbl->SetHostMode(serialPort,
PortMode_Disconnected);
- }
+ } else {
+ serialPort->vtbl->SetHostMode(serialPort,
+ PortMode_Disconnected);
+ }
- VBOX_RELEASE(serialPort);
- if (pathUtf16) {
- VBOX_UTF16_FREE(pathUtf16);
- pathUtf16 = NULL;
- }
+ VBOX_RELEASE(serialPort);
+ if (pathUtf16) {
+ VBOX_UTF16_FREE(pathUtf16);
+ pathUtf16 = NULL;
}
}
- } /* Finished:Block to attach the Serial Port to the VM */
-
- { /* Started:Block to attach the Parallel Port to the VM */
- ISystemProperties *systemProperties = NULL;
- PRUint32 parallelPortCount = 0;
- int i = 0;
-
- data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
- if (systemProperties) {
- systemProperties->vtbl->GetParallelPortCount(systemProperties,
¶llelPortCount);
- VBOX_RELEASE(systemProperties);
- systemProperties = NULL;
- }
+ }
+}
- DEBUG("Number of Parallel Ports to be connected: %d",
def->nparallels);
- DEBUG("Number of Parallel Ports available: %d", parallelPortCount);
- for (i = 0; (i < def->nparallels) && (i < parallelPortCount);
i++) {
- IParallelPort *parallelPort = NULL;
+static void
+vboxAttachParallel(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ ISystemProperties *systemProperties = NULL;
+ PRUint32 parallelPortCount = 0;
+ int i = 0;
- DEBUG("ParallelPort(%d): Type: %d", i,
def->parallels[i]->type);
- DEBUG("ParallelPort(%d): target.port: %d", i,
def->parallels[i]->target.port);
+ data->vboxObj->vtbl->GetSystemProperties(data->vboxObj,
&systemProperties);
+ if (systemProperties) {
+ systemProperties->vtbl->GetParallelPortCount(systemProperties,
+ ¶llelPortCount);
+ VBOX_RELEASE(systemProperties);
+ systemProperties = NULL;
+ }
- machine->vtbl->GetParallelPort(machine, i, ¶llelPort);
- if (parallelPort) {
- PRUnichar *pathUtf16 = NULL;
+ DEBUG("Number of Parallel Ports to be connected: %d", def->nparallels);
+ DEBUG("Number of Parallel Ports available: %d", parallelPortCount);
+ for (i = 0; (i < def->nparallels) && (i < parallelPortCount); i++)
{
+ IParallelPort *parallelPort = NULL;
- VBOX_UTF8_TO_UTF16(def->parallels[i]->data.file.path,
&pathUtf16);
+ DEBUG("ParallelPort(%d): Type: %d", i, def->parallels[i]->type);
+ DEBUG("ParallelPort(%d): target.port: %d", i,
+ def->parallels[i]->target.port);
- /* For now hard code the parallel ports to
- * LPT1 (Base Addr: 0x378 (decimal: 888), IRQ: 7)
- * LPT2 (Base Addr: 0x278 (decimal: 632), IRQ: 5)
- * TODO: make this more flexible
- */
- if ((def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_DEV) ||
- (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_PTY) ||
- (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_FILE) ||
- (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_PIPE)) {
- parallelPort->vtbl->SetPath(parallelPort, pathUtf16);
- if (i == 0) {
- parallelPort->vtbl->SetIRQ(parallelPort, 7);
- parallelPort->vtbl->SetIOBase(parallelPort, 888);
- DEBUG(" parallePort-%d irq: %d, iobase 0x%x, path:
%s",
- i, 7, 888, def->parallels[i]->data.file.path);
- } else if (i == 1) {
- parallelPort->vtbl->SetIRQ(parallelPort, 5);
- parallelPort->vtbl->SetIOBase(parallelPort, 632);
- DEBUG(" parallePort-%d irq: %d, iobase 0x%x, path:
%s",
- i, 5, 632, def->parallels[i]->data.file.path);
- }
- }
+ machine->vtbl->GetParallelPort(machine, i, ¶llelPort);
+ if (parallelPort) {
+ PRUnichar *pathUtf16 = NULL;
- /* like serial port, parallel port can't be enabled unless
- * correct IRQ and IOBase values are specified.
- */
- parallelPort->vtbl->SetEnabled(parallelPort, 1);
+ VBOX_UTF8_TO_UTF16(def->parallels[i]->data.file.path, &pathUtf16);
- VBOX_RELEASE(parallelPort);
- if (pathUtf16) {
- VBOX_UTF16_FREE(pathUtf16);
- pathUtf16 = NULL;
+ /* For now hard code the parallel ports to
+ * LPT1 (Base Addr: 0x378 (decimal: 888), IRQ: 7)
+ * LPT2 (Base Addr: 0x278 (decimal: 632), IRQ: 5)
+ * TODO: make this more flexible
+ */
+ if ((def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_DEV) ||
+ (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_PTY) ||
+ (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_FILE) ||
+ (def->parallels[i]->type == VIR_DOMAIN_CHR_TYPE_PIPE)) {
+ parallelPort->vtbl->SetPath(parallelPort, pathUtf16);
+ if (i == 0) {
+ parallelPort->vtbl->SetIRQ(parallelPort, 7);
+ parallelPort->vtbl->SetIOBase(parallelPort, 888);
+ DEBUG(" parallePort-%d irq: %d, iobase 0x%x, path: %s",
+ i, 7, 888, def->parallels[i]->data.file.path);
+ } else if (i == 1) {
+ parallelPort->vtbl->SetIRQ(parallelPort, 5);
+ parallelPort->vtbl->SetIOBase(parallelPort, 632);
+ DEBUG(" parallePort-%d irq: %d, iobase 0x%x, path: %s",
+ i, 5, 632, def->parallels[i]->data.file.path);
}
}
+
+ /* like serial port, parallel port can't be enabled unless
+ * correct IRQ and IOBase values are specified.
+ */
+ parallelPort->vtbl->SetEnabled(parallelPort, 1);
+
+ VBOX_RELEASE(parallelPort);
+ if (pathUtf16) {
+ VBOX_UTF16_FREE(pathUtf16);
+ pathUtf16 = NULL;
+ }
}
- } /* Finished:Block to attach the Parallel Port to the VM */
-
- { /* Started:Block to specify video card settings */
- if ((def->nvideos == 1) && (def->videos[0]->type ==
VIR_DOMAIN_VIDEO_TYPE_VBOX)) {
- machine->vtbl->SetVRAMSize(machine, def->videos[0]->vram);
- machine->vtbl->SetMonitorCount(machine, def->videos[0]->heads);
- if (def->videos[0]->accel) {
- machine->vtbl->SetAccelerate3DEnabled(machine,
def->videos[0]->accel->support3d);
+ }
+}
+
+static void
+vboxAttachVideo(virDomainDefPtr def, IMachine *machine)
+{
+ if ((def->nvideos == 1) &&
+ (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_VBOX)) {
+ machine->vtbl->SetVRAMSize(machine, def->videos[0]->vram);
+ machine->vtbl->SetMonitorCount(machine, def->videos[0]->heads);
+ if (def->videos[0]->accel) {
+ machine->vtbl->SetAccelerate3DEnabled(machine,
+
def->videos[0]->accel->support3d);
#if VBOX_API_VERSION >= 3001
- machine->vtbl->SetAccelerate2DVideoEnabled(machine,
def->videos[0]->accel->support2d);
+ machine->vtbl->SetAccelerate2DVideoEnabled(machine,
+
def->videos[0]->accel->support2d);
#endif /* VBOX_API_VERSION >= 3001 */
- } else {
- machine->vtbl->SetAccelerate3DEnabled(machine, 0);
+ } else {
+ machine->vtbl->SetAccelerate3DEnabled(machine, 0);
#if VBOX_API_VERSION >= 3001
- machine->vtbl->SetAccelerate2DVideoEnabled(machine, 0);
+ machine->vtbl->SetAccelerate2DVideoEnabled(machine, 0);
#endif /* VBOX_API_VERSION >= 3001 */
- }
}
- } /* Finished:Block to specify video card settings */
+ }
+}
- { /* Started:Block to attach the Remote Display to VM */
- int vrdpPresent = 0;
- int sdlPresent = 0;
- int guiPresent = 0;
- char *guiDisplay = NULL;
- char *sdlDisplay = NULL;
- int i = 0;
+static void
+vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ int vrdpPresent = 0;
+ int sdlPresent = 0;
+ int guiPresent = 0;
+ char *guiDisplay = NULL;
+ char *sdlDisplay = NULL;
+ int i = 0;
- for (i = 0; i < def->ngraphics; i++) {
- IVRDPServer *VRDPServer = NULL;
+ for (i = 0; i < def->ngraphics; i++) {
+ IVRDPServer *VRDPServer = NULL;
- if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) &&
(vrdpPresent == 0)) {
+ if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) &&
+ (vrdpPresent == 0)) {
- vrdpPresent = 1;
- machine->vtbl->GetVRDPServer(machine, &VRDPServer);
- if (VRDPServer) {
- VRDPServer->vtbl->SetEnabled(VRDPServer, PR_TRUE);
- DEBUG0("VRDP Support turned ON.");
+ vrdpPresent = 1;
+ machine->vtbl->GetVRDPServer(machine, &VRDPServer);
+ if (VRDPServer) {
+ VRDPServer->vtbl->SetEnabled(VRDPServer, PR_TRUE);
+ DEBUG0("VRDP Support turned ON.");
#if VBOX_API_VERSION < 3001
- if (def->graphics[i]->data.rdp.port) {
- VRDPServer->vtbl->SetPort(VRDPServer,
def->graphics[i]->data.rdp.port);
- DEBUG("VRDP Port changed to: %d",
def->graphics[i]->data.rdp.port);
- } else if (def->graphics[i]->data.rdp.autoport) {
- /* Setting the port to 0 will reset its value to
- * the default one which is 3389 currently
- */
- VRDPServer->vtbl->SetPort(VRDPServer, 0);
- DEBUG0("VRDP Port changed to default, which is 3389
currently");
- }
+ if (def->graphics[i]->data.rdp.port) {
+ VRDPServer->vtbl->SetPort(VRDPServer,
+ def->graphics[i]->data.rdp.port);
+ DEBUG("VRDP Port changed to: %d",
+ def->graphics[i]->data.rdp.port);
+ } else if (def->graphics[i]->data.rdp.autoport) {
+ /* Setting the port to 0 will reset its value to
+ * the default one which is 3389 currently
+ */
+ VRDPServer->vtbl->SetPort(VRDPServer, 0);
+ DEBUG0("VRDP Port changed to default, which is 3389
currently");
+ }
#else /* VBOX_API_VERSION >= 3001 */
- PRUnichar *portUtf16 = NULL;
- portUtf16 = PRUnicharFromInt(def->graphics[i]->data.rdp.port);
- VRDPServer->vtbl->SetPorts(VRDPServer, portUtf16);
- VBOX_UTF16_FREE(portUtf16);
+ PRUnichar *portUtf16 = NULL;
+ portUtf16 = PRUnicharFromInt(def->graphics[i]->data.rdp.port);
+ VRDPServer->vtbl->SetPorts(VRDPServer, portUtf16);
+ VBOX_UTF16_FREE(portUtf16);
#endif /* VBOX_API_VERSION >= 3001 */
- if (def->graphics[i]->data.rdp.replaceUser) {
- VRDPServer->vtbl->SetReuseSingleConnection(VRDPServer,
PR_TRUE);
- DEBUG0("VRDP set to reuse single connection");
- }
-
- if (def->graphics[i]->data.rdp.multiUser) {
- VRDPServer->vtbl->SetAllowMultiConnection(VRDPServer,
PR_TRUE);
- DEBUG0("VRDP set to allow multiple connection");
- }
+ if (def->graphics[i]->data.rdp.replaceUser) {
+ VRDPServer->vtbl->SetReuseSingleConnection(VRDPServer,
+ PR_TRUE);
+ DEBUG0("VRDP set to reuse single connection");
+ }
- if (def->graphics[i]->data.rdp.listenAddr) {
- PRUnichar *netAddressUtf16 = NULL;
+ if (def->graphics[i]->data.rdp.multiUser) {
+ VRDPServer->vtbl->SetAllowMultiConnection(VRDPServer,
+ PR_TRUE);
+ DEBUG0("VRDP set to allow multiple connection");
+ }
- VBOX_UTF8_TO_UTF16(def->graphics[i]->data.rdp.listenAddr,
&netAddressUtf16);
- VRDPServer->vtbl->SetNetAddress(VRDPServer,
netAddressUtf16);
- DEBUG("VRDP listen address is set to: %s",
def->graphics[i]->data.rdp.listenAddr);
+ if (def->graphics[i]->data.rdp.listenAddr) {
+ PRUnichar *netAddressUtf16 = NULL;
- VBOX_UTF16_FREE(netAddressUtf16);
- }
+ VBOX_UTF8_TO_UTF16(def->graphics[i]->data.rdp.listenAddr,
+ &netAddressUtf16);
+ VRDPServer->vtbl->SetNetAddress(VRDPServer,
+ netAddressUtf16);
+ DEBUG("VRDP listen address is set to: %s",
+ def->graphics[i]->data.rdp.listenAddr);
- VBOX_RELEASE(VRDPServer);
+ VBOX_UTF16_FREE(netAddressUtf16);
}
+
+ VBOX_RELEASE(VRDPServer);
}
+ }
- if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP)
&& (guiPresent == 0)) {
- guiPresent = 1;
- if (def->graphics[i]->data.desktop.display) {
- guiDisplay = strdup(def->graphics[i]->data.desktop.display);
- if (guiDisplay == NULL) {
- virReportOOMError();
- /* just don't go to cleanup yet as it is ok to have
- * guiDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
+ if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP)
&&
+ (guiPresent == 0)) {
+ guiPresent = 1;
+ if (def->graphics[i]->data.desktop.display) {
+ guiDisplay = strdup(def->graphics[i]->data.desktop.display);
+ if (guiDisplay == NULL) {
+ virReportOOMError();
+ /* just don't go to cleanup yet as it is ok to have
+ * guiDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
}
}
+ }
- if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) &&
(sdlPresent == 0)) {
- sdlPresent = 1;
- if (def->graphics[i]->data.sdl.display) {
- sdlDisplay = strdup(def->graphics[i]->data.sdl.display);
- if (sdlDisplay == NULL) {
- virReportOOMError();
- /* just don't go to cleanup yet as it is ok to have
- * sdlDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
+ if ((def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) &&
+ (sdlPresent == 0)) {
+ sdlPresent = 1;
+ if (def->graphics[i]->data.sdl.display) {
+ sdlDisplay = strdup(def->graphics[i]->data.sdl.display);
+ if (sdlDisplay == NULL) {
+ virReportOOMError();
+ /* just don't go to cleanup yet as it is ok to have
+ * sdlDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
}
}
}
+ }
- if ((vrdpPresent == 1) && (guiPresent == 0) && (sdlPresent == 0))
{
- /* store extradata key that frontend is set to vrdp */
- PRUnichar *keyTypeUtf16 = NULL;
- PRUnichar *valueTypeUtf16 = NULL;
+ if ((vrdpPresent == 1) && (guiPresent == 0) && (sdlPresent == 0)) {
+ /* store extradata key that frontend is set to vrdp */
+ PRUnichar *keyTypeUtf16 = NULL;
+ PRUnichar *valueTypeUtf16 = NULL;
- VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
- VBOX_UTF8_TO_UTF16("vrdp", &valueTypeUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
+ VBOX_UTF8_TO_UTF16("vrdp", &valueTypeUtf16);
- machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
+ machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
- VBOX_UTF16_FREE(keyTypeUtf16);
- VBOX_UTF16_FREE(valueTypeUtf16);
+ VBOX_UTF16_FREE(keyTypeUtf16);
+ VBOX_UTF16_FREE(valueTypeUtf16);
- } else if ((guiPresent == 0) && (sdlPresent == 1)) {
- /* store extradata key that frontend is set to sdl */
- PRUnichar *keyTypeUtf16 = NULL;
- PRUnichar *valueTypeUtf16 = NULL;
- PRUnichar *keyDislpayUtf16 = NULL;
- PRUnichar *valueDisplayUtf16 = NULL;
+ } else if ((guiPresent == 0) && (sdlPresent == 1)) {
+ /* store extradata key that frontend is set to sdl */
+ PRUnichar *keyTypeUtf16 = NULL;
+ PRUnichar *valueTypeUtf16 = NULL;
+ PRUnichar *keyDislpayUtf16 = NULL;
+ PRUnichar *valueDisplayUtf16 = NULL;
- VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
- VBOX_UTF8_TO_UTF16("sdl", &valueTypeUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
+ VBOX_UTF8_TO_UTF16("sdl", &valueTypeUtf16);
- machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
+ machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
- VBOX_UTF16_FREE(keyTypeUtf16);
- VBOX_UTF16_FREE(valueTypeUtf16);
+ VBOX_UTF16_FREE(keyTypeUtf16);
+ VBOX_UTF16_FREE(valueTypeUtf16);
- if (sdlDisplay) {
- VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
- VBOX_UTF8_TO_UTF16(sdlDisplay, &valueDisplayUtf16);
+ if (sdlDisplay) {
+ VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
+ VBOX_UTF8_TO_UTF16(sdlDisplay, &valueDisplayUtf16);
- machine->vtbl->SetExtraData(machine, keyDislpayUtf16,
valueDisplayUtf16);
+ machine->vtbl->SetExtraData(machine, keyDislpayUtf16,
+ valueDisplayUtf16);
- VBOX_UTF16_FREE(keyDislpayUtf16);
- VBOX_UTF16_FREE(valueDisplayUtf16);
- }
+ VBOX_UTF16_FREE(keyDislpayUtf16);
+ VBOX_UTF16_FREE(valueDisplayUtf16);
+ }
- } else {
- /* if all are set then default is gui, with vrdp turned on */
- PRUnichar *keyTypeUtf16 = NULL;
- PRUnichar *valueTypeUtf16 = NULL;
- PRUnichar *keyDislpayUtf16 = NULL;
- PRUnichar *valueDisplayUtf16 = NULL;
+ } else {
+ /* if all are set then default is gui, with vrdp turned on */
+ PRUnichar *keyTypeUtf16 = NULL;
+ PRUnichar *valueTypeUtf16 = NULL;
+ PRUnichar *keyDislpayUtf16 = NULL;
+ PRUnichar *valueDisplayUtf16 = NULL;
- VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
- VBOX_UTF8_TO_UTF16("gui", &valueTypeUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
+ VBOX_UTF8_TO_UTF16("gui", &valueTypeUtf16);
- machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
+ machine->vtbl->SetExtraData(machine, keyTypeUtf16, valueTypeUtf16);
- VBOX_UTF16_FREE(keyTypeUtf16);
- VBOX_UTF16_FREE(valueTypeUtf16);
+ VBOX_UTF16_FREE(keyTypeUtf16);
+ VBOX_UTF16_FREE(valueTypeUtf16);
- if (guiDisplay) {
- VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
- VBOX_UTF8_TO_UTF16(guiDisplay, &valueDisplayUtf16);
+ if (guiDisplay) {
+ VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
+ VBOX_UTF8_TO_UTF16(guiDisplay, &valueDisplayUtf16);
- machine->vtbl->SetExtraData(machine, keyDislpayUtf16,
valueDisplayUtf16);
+ machine->vtbl->SetExtraData(machine, keyDislpayUtf16,
+ valueDisplayUtf16);
- VBOX_UTF16_FREE(keyDislpayUtf16);
- VBOX_UTF16_FREE(valueDisplayUtf16);
- }
+ VBOX_UTF16_FREE(keyDislpayUtf16);
+ VBOX_UTF16_FREE(valueDisplayUtf16);
}
+ }
- VIR_FREE(guiDisplay);
- VIR_FREE(sdlDisplay);
+ VIR_FREE(guiDisplay);
+ VIR_FREE(sdlDisplay);
+}
- } /* Finished:Block to attach the Remote Display to VM */
+static void
+vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
+{
+ IUSBController *USBController = NULL;
+ int i = 0;
+ bool isUSB = false;
+
+ if (def->nhostdevs == 0)
+ return;
+
+ /* Loop through the devices first and see if you
+ * have a USB Device, only if you have one then
+ * start the USB controller else just proceed as
+ * usual
+ */
+ for (i = 0; i < def->nhostdevs; i++) {
+ if (def->hostdevs[i]->mode ==
+ VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+ if (def->hostdevs[i]->source.subsys.type ==
+ VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+ if (def->hostdevs[i]->source.subsys.u.usb.vendor ||
+ def->hostdevs[i]->source.subsys.u.usb.product) {
+ DEBUG("USB Device detected, VendorId:0x%x,
ProductId:0x%x",
+ def->hostdevs[i]->source.subsys.u.usb.vendor,
+ def->hostdevs[i]->source.subsys.u.usb.product);
+ isUSB = true;
+ break;
+ }
+ }
+ }
+ }
+
+ if (isUSB) {
+ /* First Start the USB Controller and then loop
+ * to attach USB Devices to it
+ */
+ machine->vtbl->GetUSBController(machine, &USBController);
+ if (USBController) {
+ USBController->vtbl->SetEnabled(USBController, 1);
+ USBController->vtbl->SetEnabledEhci(USBController, 1);
- { /* Started:Block to attach USB Devices to VM */
- if (def->nhostdevs > 0) {
- IUSBController *USBController = NULL;
- int i = 0, isUSB = 0;
- /* Loop through the devices first and see if you
- * have a USB Device, only if you have one then
- * start the USB controller else just proceed as
- * usual
- */
for (i = 0; i < def->nhostdevs; i++) {
if (def->hostdevs[i]->mode ==
- VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+ VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
if (def->hostdevs[i]->source.subsys.type ==
- VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- if (def->hostdevs[i]->source.subsys.u.usb.vendor ||
- def->hostdevs[i]->source.subsys.u.usb.product) {
- DEBUG("USB Device detected, VendorId:0x%x,
ProductId:0x%x",
- def->hostdevs[i]->source.subsys.u.usb.vendor,
- def->hostdevs[i]->source.subsys.u.usb.product);
- isUSB++;
- }
- }
- }
- }
+ VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- if (isUSB > 0) {
- /* First Start the USB Controller and then loop
- * to attach USB Devices to it
- */
- machine->vtbl->GetUSBController(machine, &USBController);
- if (USBController) {
- USBController->vtbl->SetEnabled(USBController, 1);
- USBController->vtbl->SetEnabledEhci(USBController, 1);
-
- for (i = 0; i < def->nhostdevs; i++) {
- if (def->hostdevs[i]->mode ==
- VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- if (def->hostdevs[i]->source.subsys.type ==
- VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
-
- char filtername[11] = {0};
- PRUnichar *filternameUtf16 = NULL;
- IUSBDeviceFilter *filter = NULL;
-
- /* Assuming can't have more then 9999 devices so
- * restricting to %04d
- */
- sprintf(filtername, "filter%04d", i);
- VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
-
-
USBController->vtbl->CreateDeviceFilter(USBController,
- filternameUtf16,
- &filter);
- VBOX_UTF16_FREE(filternameUtf16);
-
- if (filter &&
- (def->hostdevs[i]->source.subsys.u.usb.vendor
||
- def->hostdevs[i]->source.subsys.u.usb.product))
{
-
- PRUnichar *vendorIdUtf16 = NULL;
- char vendorId[40] = {0};
- PRUnichar *productIdUtf16 = NULL;
- char productId[40] = {0};
-
- if
(def->hostdevs[i]->source.subsys.u.usb.vendor) {
- sprintf(vendorId, "%x",
def->hostdevs[i]->source.subsys.u.usb.vendor);
- VBOX_UTF8_TO_UTF16(vendorId,
&vendorIdUtf16);
- filter->vtbl->SetVendorId(filter,
vendorIdUtf16);
- VBOX_UTF16_FREE(vendorIdUtf16);
- }
- if
(def->hostdevs[i]->source.subsys.u.usb.product) {
- sprintf(productId, "%x",
def->hostdevs[i]->source.subsys.u.usb.product);
- VBOX_UTF8_TO_UTF16(productId,
&productIdUtf16);
- filter->vtbl->SetProductId(filter,
productIdUtf16);
- VBOX_UTF16_FREE(productIdUtf16);
- }
- filter->vtbl->SetActive(filter, 1);
-
USBController->vtbl->InsertDeviceFilter(USBController,
- i,
- filter);
- VBOX_RELEASE(filter);
- }
+ char filtername[11] = {0};
+ PRUnichar *filternameUtf16 = NULL;
+ IUSBDeviceFilter *filter = NULL;
+ /* Assuming can't have more then 9999 devices so
+ * restricting to %04d
+ */
+ sprintf(filtername, "filter%04d", i);
+ VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+
+ USBController->vtbl->CreateDeviceFilter(USBController,
+ filternameUtf16,
+ &filter);
+ VBOX_UTF16_FREE(filternameUtf16);
+
+ if (filter &&
+ (def->hostdevs[i]->source.subsys.u.usb.vendor ||
+ def->hostdevs[i]->source.subsys.u.usb.product)) {
+
+ PRUnichar *vendorIdUtf16 = NULL;
+ char vendorId[40] = {0};
+ PRUnichar *productIdUtf16 = NULL;
+ char productId[40] = {0};
+
+ if (def->hostdevs[i]->source.subsys.u.usb.vendor) {
+ sprintf(vendorId, "%x",
def->hostdevs[i]->source.subsys.u.usb.vendor);
+ VBOX_UTF8_TO_UTF16(vendorId, &vendorIdUtf16);
+ filter->vtbl->SetVendorId(filter, vendorIdUtf16);
+ VBOX_UTF16_FREE(vendorIdUtf16);
+ }
+ if (def->hostdevs[i]->source.subsys.u.usb.product) {
+ sprintf(productId, "%x",
def->hostdevs[i]->source.subsys.u.usb.product);
+ VBOX_UTF8_TO_UTF16(productId, &productIdUtf16);
+ filter->vtbl->SetProductId(filter,
+ productIdUtf16);
+ VBOX_UTF16_FREE(productIdUtf16);
}
+ filter->vtbl->SetActive(filter, 1);
+ USBController->vtbl->InsertDeviceFilter(USBController,
+ i,
+ filter);
+ VBOX_RELEASE(filter);
}
+
}
- VBOX_RELEASE(USBController);
}
}
+ VBOX_RELEASE(USBController);
+ }
+ }
+}
+
+static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
+ VBOX_OBJECT_CHECK(conn, virDomainPtr, NULL);
+ IMachine *machine = NULL;
+ IBIOSSettings *bios = NULL;
+ vboxIID *iid = NULL;
+ vboxIID *mchiid = NULL;
+ virDomainDefPtr def = NULL;
+ PRUnichar *machineNameUtf16 = NULL;
+#if VBOX_API_VERSION >= 3002
+ PRBool override = PR_FALSE;
+#endif
+ nsresult rc;
+
+ if (!(def = virDomainDefParseString(data->caps, xml,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto cleanup;
+ }
+
+#if VBOX_API_VERSION == 2002
+ if (VIR_ALLOC(iid) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+#endif
+
+ VBOX_UTF8_TO_UTF16(def->name, &machineNameUtf16);
+ vboxIIDFromUUID(def->uuid, iid);
+#if VBOX_API_VERSION < 3002
+ rc = data->vboxObj->vtbl->CreateMachine(data->vboxObj,
+ machineNameUtf16,
+ NULL,
+ NULL,
+ iid,
+ &machine);
+#else /* VBOX_API_VERSION >= 3002 */
+ rc = data->vboxObj->vtbl->CreateMachine(data->vboxObj,
+ machineNameUtf16,
+ NULL,
+ NULL,
+ iid,
+ override,
+ &machine);
+#endif /* VBOX_API_VERSION >= 3002 */
+ VBOX_UTF16_FREE(machineNameUtf16);
+
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not define a domain, rc=%08x"), (unsigned)rc);
+ goto cleanup;
+ }
+
+ rc = machine->vtbl->SetMemorySize(machine, def->memory / 1024);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not set the memory size of the domain to: %lu Kb, "
+ "rc=%08x"),
+ def->memory, (unsigned)rc);
+ }
+
+ rc = machine->vtbl->SetCPUCount(machine, def->vcpus);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not set the number of virtual CPUs to: %lu,
rc=%08x"),
+ def->vcpus, (unsigned)rc);
+ }
+
+#if VBOX_API_VERSION < 3001
+ rc = machine->vtbl->SetPAEEnabled(machine, (def->features) &
+ (1 << VIR_DOMAIN_FEATURE_PAE));
+#elif VBOX_API_VERSION == 3001
+ rc = machine->vtbl->SetCpuProperty(machine, CpuPropertyType_PAE,
+ (def->features) &
+ (1 << VIR_DOMAIN_FEATURE_PAE));
+#elif VBOX_API_VERSION >= 3002
+ rc = machine->vtbl->SetCPUProperty(machine, CPUPropertyType_PAE,
+ (def->features) &
+ (1 << VIR_DOMAIN_FEATURE_PAE));
+#endif
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not change PAE status to: %s, rc=%08x"),
+ ((def->features) & (1 << VIR_DOMAIN_FEATURE_PAE))
+ ? _("Enabled") : _("Disabled"), (unsigned)rc);
+ }
+
+ machine->vtbl->GetBIOSSettings(machine, &bios);
+ if (bios) {
+ rc = bios->vtbl->SetACPIEnabled(bios, (def->features) &
+ (1 << VIR_DOMAIN_FEATURE_ACPI));
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not change ACPI status to: %s, rc=%08x"),
+ ((def->features) & (1 << VIR_DOMAIN_FEATURE_ACPI))
+ ? _("Enabled") : _("Disabled"), (unsigned)rc);
+ }
+ rc = bios->vtbl->SetIOAPICEnabled(bios, (def->features) &
+ (1 << VIR_DOMAIN_FEATURE_APIC));
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not change APIC status to: %s, rc=%08x"),
+ ((def->features) & (1 << VIR_DOMAIN_FEATURE_APIC))
+ ? _("Enabled") : _("Disabled"), (unsigned)rc);
}
- } /* Finished:Block to attach USB Devices to VM */
+ VBOX_RELEASE(bios);
+ }
+
+ /* Register the machine before attaching other devices to it */
+ rc = data->vboxObj->vtbl->RegisterMachine(data->vboxObj, machine);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not define a domain, rc=%08x"), (unsigned)rc);
+ goto cleanup;
+ }
+
+ /* Get the uuid of the machine, currently it is immutable
+ * object so open a session to it and get it back, so that
+ * you can make changes to the machine setting
+ */
+ machine->vtbl->GetId(machine, &mchiid);
+ data->vboxObj->vtbl->OpenSession(data->vboxObj, data->vboxSession,
mchiid);
+ data->vboxSession->vtbl->GetMachine(data->vboxSession, &machine);
+
+ vboxSetBootDeviceOrder(def, data, machine);
+ vboxAttachDrives(def, data, machine);
+ vboxAttachSound(def, machine);
+ vboxAttachNetwork(def, data, machine);
+ vboxAttachSerial(def, data, machine);
+ vboxAttachParallel(def, data, machine);
+ vboxAttachVideo(def, machine);
+ vboxAttachDisplay(def, data, machine);
+ vboxAttachUSB(def, data, machine);
/* Save the machine settings made till now and close the
* session. also free up the mchiid variable used.
--
1.7.2.2
6:25 a.m.
New subject: [libvirt] [PATCH 2/7] vbox: factor a large function
2010/9/1 Eric Blake <eblake(a)redhat.com>:
* src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Split...
(vboxSetBootDeviceOrder, vboxAttachDrives, vboxAttachSound)
(vboxAttachNetwork, vboxAttachSerial, vboxAttachParallel)
(vboxAttachVideo, vboxAttachDisplay, vboxAttachUSB): ...into new
helper functions.
---
Should be no functional changes - just breaking up a HUGE function
into more manageable chunks with less indentation.
In reviewing this, it may help to apply the patch and use
'git diff -b'.
That helps a lot and it reveals that a major part of the remaining
diff is just breaking long lines. You could have split that into a
separate patch, but it's okay as it is.
ACK.
Mathias
4:53 p.m.
New subject: [libvirt] [PATCH 2/7] vbox: factor a large function
On 09/02/2010 05:25 AM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> * src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Split...
> (vboxSetBootDeviceOrder, vboxAttachDrives, vboxAttachSound)
> (vboxAttachNetwork, vboxAttachSerial, vboxAttachParallel)
> (vboxAttachVideo, vboxAttachDisplay, vboxAttachUSB): ...into new
> helper functions.
> ---
>
> Should be no functional changes - just breaking up a HUGE function
> into more manageable chunks with less indentation.
>
> In reviewing this, it may help to apply the patch and use
> 'git diff -b'.
>
That helps a lot and it reveals that a major part of the remaining
diff is just breaking long lines. You could have split that into a
separate patch, but it's okay as it is.
Oh well; next time I do a major refactor, I'll try to remember to split
it into two steps (motion, then indenting/line length) for review, even
if I then squash them for actual pushing.
ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 3/7] vbox: avoid problematic uses of sprintf
* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
---
This removes all use of sprintf in vbox. The first 3 use virAsprintf
(DISPLAY may be arbitrarily long, and while we are unlikely to hit
9999 devices, it's better to be safe than to risk silent buffer
overflow); the remaining two are sized appropriately (actually, they
are sized too large, the real boundary size would be sizeof(int)*2+1
rather than 40); I felt better using snprintf rather than sprintf.
This doesn't address the fact that vbox doesn't really have very good
OOM handling (ie. it keeps on trying, although after the first OOM,
it will likely get another one); but that is an independent issue.
src/vbox/vbox_tmpl.c | 39 +++++++++++++++++++++++++++------------
1 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3e8ff23..f50a12e 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -3161,7 +3161,6 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID
*iid)
PRUnichar *valueDisplayUtf16 = NULL;
char *valueDisplayUtf8 = NULL;
IProgress *progress = NULL;
- char displayutf8[32] = {0};
PRUnichar *env = NULL;
PRUnichar *sessionType = NULL;
nsresult rc;
@@ -3241,8 +3240,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine,
vboxIID *iid)
if (guiPresent) {
if (guiDisplay) {
- sprintf(displayutf8, "DISPLAY=%.24s", guiDisplay);
- VBOX_UTF8_TO_UTF16(displayutf8, &env);
+ char *displayutf8;
+ if (virAsprintf(&displayutf8, "DISPLAY=%s", guiDisplay) <
0)
+ virReportOOMError();
+ else {
+ VBOX_UTF8_TO_UTF16(displayutf8, &env);
+ VIR_FREE(displayutf8);
+ }
VIR_FREE(guiDisplay);
}
@@ -3251,8 +3255,13 @@ vboxStartMachine(virDomainPtr dom, int i, IMachine *machine,
vboxIID *iid)
if (sdlPresent) {
if (sdlDisplay) {
- sprintf(displayutf8, "DISPLAY=%.24s", sdlDisplay);
- VBOX_UTF8_TO_UTF16(displayutf8, &env);
+ char *displayutf8;
+ if (virAsprintf(&displayutf8, "DISPLAY=%s", sdlDisplay) <
0)
+ virReportOOMError();
+ else {
+ VBOX_UTF8_TO_UTF16(displayutf8, &env);
+ VIR_FREE(displayutf8);
+ }
VIR_FREE(sdlDisplay);
}
@@ -4457,15 +4466,19 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
if (def->hostdevs[i]->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- char filtername[11] = {0};
+ char *filtername = NULL;
PRUnichar *filternameUtf16 = NULL;
IUSBDeviceFilter *filter = NULL;
- /* Assuming can't have more then 9999 devices so
- * restricting to %04d
+ /* Zero pad for nice alignment when fewer than 9999
+ * devices.
*/
- sprintf(filtername, "filter%04d", i);
- VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+ if (virAsprintf(&filtername, "filter%04d", i) <
0) {
+ virReportOOMError();
+ } else {
+ VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+ VIR_FREE(filtername);
+ }
USBController->vtbl->CreateDeviceFilter(USBController,
filternameUtf16,
@@ -4482,13 +4495,15 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
char productId[40] = {0};
if (def->hostdevs[i]->source.subsys.u.usb.vendor) {
- sprintf(vendorId, "%x",
def->hostdevs[i]->source.subsys.u.usb.vendor);
+ snprintf(vendorId, sizeof(vendorId), "%x",
+
def->hostdevs[i]->source.subsys.u.usb.vendor);
VBOX_UTF8_TO_UTF16(vendorId, &vendorIdUtf16);
filter->vtbl->SetVendorId(filter, vendorIdUtf16);
VBOX_UTF16_FREE(vendorIdUtf16);
}
if (def->hostdevs[i]->source.subsys.u.usb.product) {
- sprintf(productId, "%x",
def->hostdevs[i]->source.subsys.u.usb.product);
+ snprintf(productId, sizeof(productId), "%x",
+
def->hostdevs[i]->source.subsys.u.usb.product);
VBOX_UTF8_TO_UTF16(productId, &productIdUtf16);
filter->vtbl->SetProductId(filter,
productIdUtf16);
--
1.7.2.2
6:43 a.m.
New subject: [libvirt] [PATCH 3/7] vbox: avoid problematic uses of sprintf
2010/9/1 Eric Blake <eblake(a)redhat.com>:
* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
---
This removes all use of sprintf in vbox. The first 3 use virAsprintf
(DISPLAY may be arbitrarily long, and while we are unlikely to hit
9999 devices, it's better to be safe than to risk silent buffer
overflow); the remaining two are sized appropriately (actually, they
are sized too large, the real boundary size would be sizeof(int)*2+1
rather than 40); I felt better using snprintf rather than sprintf.
This doesn't address the fact that vbox doesn't really have very good
OOM handling (ie. it keeps on trying, although after the first OOM,
it will likely get another one); but that is an independent issue.
@@ -4457,15 +4466,19 @@ vboxAttachUSB(virDomainDefPtr def,
vboxGlobalData *data, IMachine *machine)
if (def->hostdevs[i]->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- char filtername[11] = {0};
+ char *filtername = NULL;
PRUnichar *filternameUtf16 = NULL;
IUSBDeviceFilter *filter = NULL;
- /* Assuming can't have more then 9999 devices so
- * restricting to %04d
+ /* Zero pad for nice alignment when fewer than 9999
+ * devices.
*/
- sprintf(filtername, "filter%04d", i);
- VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+ if (virAsprintf(&filtername, "filter%04d", i) <
0) {
+ virReportOOMError();
+ } else {
+ VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
+ VIR_FREE(filtername);
+ }
USBController->vtbl->CreateDeviceFilter(USBController,
filternameUtf16,
In case virAsprintf fails CreateDeviceFilter will be called with
filternameUtf16 = NULL, that might trigger a segfault.
Matthias
5:23 p.m.
New subject: [libvirt] [PATCH 3/7] vbox: avoid problematic uses of sprintf
On 09/02/2010 05:43 AM, Matthias Bolte wrote:
> - /* Assuming can't have more then
9999 devices so
> - * restricting to %04d
> + /* Zero pad for nice alignment when fewer than 9999
> + * devices.
> */
> - sprintf(filtername, "filter%04d", i);
> - VBOX_UTF8_TO_UTF16(filtername,&filternameUtf16);
> + if (virAsprintf(&filtername, "filter%04d",
i)< 0) {
> + virReportOOMError();
> + } else {
> + VBOX_UTF8_TO_UTF16(filtername,&filternameUtf16);
> + VIR_FREE(filtername);
> + }
>
>
USBController->vtbl->CreateDeviceFilter(USBController,
> filternameUtf16,
In case virAsprintf fails CreateDeviceFilter will be called with
filternameUtf16 = NULL, that might trigger a segfault.
Good catch. I'll respin this patch as part of my round 3 cleanups (more
openvz_driver changes, and virsh.c, still to come). It may be another
day or two...
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 4/7] network: use virAsprintf when appropriate
* src/conf/network_conf.c (virNetworkAllocateBridge): Avoid
limited buffer from snprintf.
---
Why print to a fixed-width buffer to then just strdup it later,
when we can print to a malloc'd buffer in the first place. Besides,
I couldn't easily guarantee if the buffer was large enough or if
it would contain arbitrary user input.
src/conf/network_conf.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 347fc0b..4c0248c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -891,17 +891,14 @@ char *virNetworkAllocateBridge(const virNetworkObjListPtr nets,
template = "virbr%d";
do {
- char try[50];
-
- snprintf(try, sizeof(try), template, id);
-
- if (!virNetworkBridgeInUse(nets, try, NULL)) {
- if (!(newname = strdup(try))) {
- virReportOOMError();
- return NULL;
- }
+ if (virAsprintf(&newname, template, id) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+ if (!virNetworkBridgeInUse(nets, newname, NULL)) {
return newname;
}
+ VIR_FREE(newname);
id++;
} while (id <= MAX_BRIDGE_ID);
--
1.7.2.2
4:20 p.m.
New subject: [libvirt] [PATCH 4/7] network: use virAsprintf when appropriate
2010/9/1 Eric Blake <eblake(a)redhat.com>:
* src/conf/network_conf.c (virNetworkAllocateBridge): Avoid
limited buffer from snprintf.
---
Why print to a fixed-width buffer to then just strdup it later,
when we can print to a malloc'd buffer in the first place. Besides,
I couldn't easily guarantee if the buffer was large enough or if
it would contain arbitrary user input.
src/conf/network_conf.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 347fc0b..4c0248c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -891,17 +891,14 @@ char *virNetworkAllocateBridge(const virNetworkObjListPtr nets,
template = "virbr%d";
do {
- char try[50];
-
- snprintf(try, sizeof(try), template, id);
-
- if (!virNetworkBridgeInUse(nets, try, NULL)) {
- if (!(newname = strdup(try))) {
- virReportOOMError();
- return NULL;
- }
+ if (virAsprintf(&newname, template, id) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+ if (!virNetworkBridgeInUse(nets, newname, NULL)) {
return newname;
}
+ VIR_FREE(newname);
id++;
} while (id <= MAX_BRIDGE_ID);
--
1.7.2.2
ACK.
Matthias
6:01 p.m.
New subject: [libvirt] [PATCH 4/7] network: use virAsprintf when appropriate
On 09/01/2010 03:20 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> * src/conf/network_conf.c (virNetworkAllocateBridge): Avoid
> limited buffer from snprintf.
> ---
>
> Why print to a fixed-width buffer to then just strdup it later,
> when we can print to a malloc'd buffer in the first place. Besides,
> I couldn't easily guarantee if the buffer was large enough or if
> it would contain arbitrary user input.
>
ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 5/7] lxc: avoid large stacks with veth creation
* src/lxc/veth.h (vethCreate): Change prototype.
* src/lxc/veth.c (vethCreate): Always malloc veth2, and allocate
veth1 if needed.
(getFreeVethName): Adjust signature, and use virAsprintf.
* src/lxc/lxc_driver.c (lxcSetupInterfaces): Adjust caller.
---
This issue crossed file boundaries. It was a bit tricky, since
vethCreate was used in two patterns - one where the parent name was
already known, and another where the parent name is picked as the
first available option. But the end result avoids strdup'ing a
fixed-width buffer, and I think I correctly avoided any leaks (in
lxcSetupInterfaces, once a string is transferred to *veths, then
returning failure will cause that string to be freed in the caller).
It also gets rid of a PATH_MAX stack over-allocation.
src/lxc/lxc_driver.c | 32 ++++++++-----------
src/lxc/veth.c | 84 ++++++++++++++++++++++++++++++-------------------
src/lxc/veth.h | 6 ++--
3 files changed, 67 insertions(+), 55 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6fe20b1..326fee6 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -812,14 +812,16 @@ static int lxcSetupInterfaces(virConnectPtr conn,
return -1;
for (i = 0 ; i < def->nnets ; i++) {
- char parentVeth[PATH_MAX] = "";
- char containerVeth[PATH_MAX] = "";
+ char *parentVeth;
+ char *containerVeth = NULL;
switch (def->nets[i]->type) {
case VIR_DOMAIN_NET_TYPE_NETWORK:
{
- virNetworkPtr network = virNetworkLookupByName(conn,
-
def->nets[i]->data.network.name);
+ virNetworkPtr network;
+
+ network = virNetworkLookupByName(conn,
+ def->nets[i]->data.network.name);
if (!network) {
goto error_exit;
}
@@ -852,31 +854,23 @@ static int lxcSetupInterfaces(virConnectPtr conn,
}
DEBUG0("calling vethCreate()");
- if (NULL != def->nets[i]->ifname) {
- strcpy(parentVeth, def->nets[i]->ifname);
- }
- DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth);
- if (vethCreate(parentVeth, PATH_MAX, containerVeth, PATH_MAX) < 0)
+ parentVeth = def->nets[i]->ifname;
+ if (vethCreate(&parentVeth, &containerVeth) < 0)
goto error_exit;
+ DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth);
if (NULL == def->nets[i]->ifname) {
- def->nets[i]->ifname = strdup(parentVeth);
+ def->nets[i]->ifname = parentVeth;
}
+
if (VIR_REALLOC_N(*veths, (*nveths)+1) < 0) {
virReportOOMError();
+ VIR_FREE(containerVeth);
goto error_exit;
}
- if (((*veths)[(*nveths)] = strdup(containerVeth)) == NULL) {
- virReportOOMError();
- goto error_exit;
- }
+ (*veths)[(*nveths)] = containerVeth;
(*nveths)++;
- if (NULL == def->nets[i]->ifname) {
- virReportOOMError();
- goto error_exit;
- }
-
{
char macaddr[VIR_MAC_STRING_BUFLEN];
virFormatMacAddr(def->nets[i]->mac, macaddr);
diff --git a/src/lxc/veth.c b/src/lxc/veth.c
index 5f038d6..14cfaa2 100644
--- a/src/lxc/veth.c
+++ b/src/lxc/veth.c
@@ -13,6 +13,7 @@
#include <config.h>
#include <string.h>
+#include <stdbool.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/wait.h>
@@ -33,42 +34,46 @@
/* Functions */
/**
* getFreeVethName:
- * @veth: name for veth device (NULL to find first open)
- * @maxLen: max length of veth name
+ * @veth: pointer to store returned name for veth device
* @startDev: device number to start at (x in vethx)
*
* Looks in /sys/class/net/ to find the first available veth device
* name.
*
- * Returns 0 on success or -1 in case of error
+ * Returns non-negative device number on success or -1 in case of error
*/
-static int getFreeVethName(char *veth, int maxLen, int startDev)
+static int getFreeVethName(char **veth, int startDev)
{
- int rc = -1;
int devNum = startDev-1;
- char path[PATH_MAX];
+ char *path = NULL;
do {
+ VIR_FREE(path);
++devNum;
- snprintf(path, PATH_MAX, "/sys/class/net/veth%d/", devNum);
+ if (virAsprintf(&path, "/sys/class/net/veth%d/", devNum) < 0) {
+ virReportOOMError();
+ return -1;
+ }
} while (virFileExists(path));
+ VIR_FREE(path);
- snprintf(veth, maxLen, "veth%d", devNum);
-
- rc = devNum;
-
- return rc;
+ if (virAsprintf(veth, "veth%d", devNum) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+ return devNum;
}
/**
* vethCreate:
- * @veth1: name for one end of veth pair
- * @veth1MaxLen: max length of veth1 name
- * @veth2: name for one end of veth pair
- * @veth2MaxLen: max length of veth1 name
+ * @veth1: pointer to name for parent end of veth pair
+ * @veth2: pointer to return name for container end of veth pair
*
* Creates a veth device pair using the ip command:
* ip link add veth1 type veth peer name veth2
+ * If veth1 points to NULL on entry, it will be a valid interface on
+ * return. veth2 should point to NULL on entry.
+ *
* NOTE: If veth1 and veth2 names are not specified, ip will auto assign
* names. There seems to be two problems here -
* 1) There doesn't seem to be a way to determine the names of the
@@ -78,43 +83,56 @@ static int getFreeVethName(char *veth, int maxLen, int startDev)
* 2) Once one of the veth devices is moved to another namespace, it
* is no longer visible in the parent namespace. This seems to
* confuse the name assignment causing it to fail with File exists.
- * Because of these issues, this function currently forces the caller
- * to fully specify the veth device names.
+ * Because of these issues, this function currently allocates names
+ * prior to using the ip command, and returns any allocated names
+ * to the caller.
*
* Returns 0 on success or -1 in case of error
*/
-int vethCreate(char* veth1, int veth1MaxLen,
- char* veth2, int veth2MaxLen)
+int vethCreate(char** veth1, char** veth2)
{
int rc;
const char *argv[] = {
- "ip", "link", "add", veth1, "type",
"veth", "peer", "name", veth2, NULL
+ "ip", "link", "add", NULL, "type",
"veth", "peer", "name", NULL, NULL
};
int cmdResult = 0;
int vethDev = 0;
+ bool veth1_alloc = false;
- DEBUG("veth1: %s veth2: %s", veth1, veth2);
+ DEBUG("veth1: %s veth2: %s", NULLSTR(*veth1), NULLSTR(*veth2));
- while ((1 > strlen(veth1)) || STREQ(veth1, veth2)) {
- vethDev = getFreeVethName(veth1, veth1MaxLen, 0);
- ++vethDev;
- DEBUG("Assigned veth1: %s", veth1);
+ if (*veth1 == NULL) {
+ vethDev = getFreeVethName(veth1, vethDev);
+ if (vethDev < 0)
+ return vethDev;
+ DEBUG("Assigned veth1: %s", *veth1);
+ veth1_alloc = true;
}
-
- while ((1 > strlen(veth2)) || STREQ(veth1, veth2)) {
- vethDev = getFreeVethName(veth2, veth2MaxLen, vethDev);
- ++vethDev;
- DEBUG("Assigned veth2: %s", veth2);
+ argv[3] = *veth1;
+
+ while (*veth2 == NULL || STREQ(*veth1, *veth2)) {
+ VIR_FREE(*veth2);
+ vethDev = getFreeVethName(veth2, vethDev + 1);
+ if (vethDev < 0) {
+ if (veth1_alloc)
+ VIR_FREE(*veth1);
+ return vethDev;
+ }
+ DEBUG("Assigned veth2: %s", *veth2);
}
+ argv[8] = *veth2;
- DEBUG("veth1: %s veth2: %s", veth1, veth2);
+ DEBUG("veth1: %s veth2: %s", *veth1, *veth2);
rc = virRun(argv, &cmdResult);
if (rc != 0 ||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
vethError(VIR_ERR_INTERNAL_ERROR,
_("Failed to create veth device pair '%s', '%s':
%d"),
- veth1, veth2, WEXITSTATUS(cmdResult));
+ *veth1, *veth2, WEXITSTATUS(cmdResult));
+ if (veth1_alloc)
+ VIR_FREE(*veth1);
+ VIR_FREE(*veth2);
rc = -1;
}
diff --git a/src/lxc/veth.h b/src/lxc/veth.h
index 1ec1ec8..f50a939 100644
--- a/src/lxc/veth.h
+++ b/src/lxc/veth.h
@@ -1,6 +1,7 @@
/*
* veth.h: Interface to tools for managing veth pairs
*
+ * Copyright (C) 2010 Red Hat, Inc.
* Copyright IBM Corp. 2008
*
* See COPYING.LIB for the License of this software
@@ -16,9 +17,8 @@
# include "internal.h"
/* Function declarations */
-int vethCreate(char* veth1, int veth1MaxLen, char* veth2,
- int veth2MaxLen)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+int vethCreate(char** veth1, char** veth2)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int vethDelete(const char* veth)
ATTRIBUTE_NONNULL(1);
int vethInterfaceUpOrDown(const char* veth, int upOrDown)
--
1.7.2.2
7:16 a.m.
New subject: [libvirt] [PATCH 5/7] lxc: avoid large stacks with veth creation
2010/9/1 Eric Blake <eblake(a)redhat.com>:
* src/lxc/veth.h (vethCreate): Change prototype.
* src/lxc/veth.c (vethCreate): Always malloc veth2, and allocate
veth1 if needed.
(getFreeVethName): Adjust signature, and use virAsprintf.
* src/lxc/lxc_driver.c (lxcSetupInterfaces): Adjust caller.
---
This issue crossed file boundaries. It was a bit tricky, since
vethCreate was used in two patterns - one where the parent name was
already known, and another where the parent name is picked as the
first available option. But the end result avoids strdup'ing a
fixed-width buffer, and I think I correctly avoided any leaks (in
lxcSetupInterfaces, once a string is transferred to *veths, then
returning failure will cause that string to be freed in the caller).
It also gets rid of a PATH_MAX stack over-allocation.
Looks good, ACK.
Matthias
4:53 p.m.
New subject: [libvirt] [PATCH 5/7] lxc: avoid large stacks with veth creation
On 09/02/2010 06:16 AM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> * src/lxc/veth.h (vethCreate): Change prototype.
> * src/lxc/veth.c (vethCreate): Always malloc veth2, and allocate
> veth1 if needed.
> (getFreeVethName): Adjust signature, and use virAsprintf.
> * src/lxc/lxc_driver.c (lxcSetupInterfaces): Adjust caller.
> ---
>
> This issue crossed file boundaries. It was a bit tricky, since
> vethCreate was used in two patterns - one where the parent name was
> already known, and another where the parent name is picked as the
> first available option. But the end result avoids strdup'ing a
> fixed-width buffer, and I think I correctly avoided any leaks (in
> lxcSetupInterfaces, once a string is transferred to *veths, then
> returning failure will cause that string to be freed in the caller).
> It also gets rid of a PATH_MAX stack over-allocation.
>
Looks good, ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 6/7] openvz: formatting cleanups
* src/openvz/openvz_conf.c: Whitespace fixes.
* src/openvz/openvz_driver.c: Likewise.
---
Should just be formatting, no content change.
openvz has other problems, like its use of popen (totally unsafe);
so I'll be fixing it some more when I get to the virCommand patch
series.
src/openvz/openvz_conf.c | 70 ++++++++++++++++++++++----------------------
src/openvz/openvz_driver.c | 23 +++++++-------
2 files changed, 47 insertions(+), 46 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index b52f4ac..356c7f0 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -64,7 +64,7 @@ strtoI(const char *str)
int val;
if (virStrToLong_i(str, NULL, 10, &val) < 0)
- return 0 ;
+ return 0;
return val;
}
@@ -338,7 +338,7 @@ openvz_replace(const char* str,
from_len = strlen(from);
to_len = strlen(to);
- while((offset = strstr(str_start, from)))
+ while ((offset = strstr(str_start, from)))
{
virBufferAdd(&buf, str_start, offset-str_start);
virBufferAdd(&buf, to, to_len);
@@ -447,7 +447,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
return -1;
}
- while(!feof(fp)) {
+ while (!feof(fp)) {
if (fscanf(fp, "%d %s\n", &veid, status) != 2) {
if (feof(fp))
break;
@@ -556,7 +556,7 @@ openvzWriteConfigParam(const char * conf_file, const char *param,
const char *va
{
char * temp_file = NULL;
int fd = -1, temp_fd = -1;
- char line[PATH_MAX] ;
+ char line[PATH_MAX];
if (virAsprintf(&temp_file, "%s.tmp", conf_file)<0) {
virReportOOMError();
@@ -572,7 +572,7 @@ openvzWriteConfigParam(const char * conf_file, const char *param,
const char *va
goto error;
}
- while(1) {
+ while (1) {
if (openvz_readline(fd, line, sizeof(line)) <= 0)
break;
@@ -606,7 +606,7 @@ error:
close(fd);
if (temp_fd != -1)
close(temp_fd);
- if(temp_file)
+ if (temp_file)
unlink(temp_file);
VIR_FREE(temp_file);
return -1;
@@ -626,9 +626,9 @@ openvzWriteVPSConfigParam(int vpsid, const char *param, const char
*value)
static int
openvzReadConfigParam(const char * conf_file ,const char * param, char *value, int
maxlen)
{
- char line[PATH_MAX] ;
+ char line[PATH_MAX];
int ret, found = 0;
- int fd ;
+ int fd;
char * sf, * token;
char *saveptr = NULL;
@@ -638,16 +638,16 @@ openvzReadConfigParam(const char * conf_file ,const char * param,
char *value, i
if (fd == -1)
return -1;
- while(1) {
+ while (1) {
ret = openvz_readline(fd, line, sizeof(line));
- if(ret <= 0)
+ if (ret <= 0)
break;
saveptr = NULL;
if (STREQLEN(line, param, strlen(param))) {
sf = line;
sf += strlen(param);
if (sf[0] == '=' && sf[1] != '\0' ) {
- sf ++;
+ sf++;
if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL)
{
if (virStrcpy(value, token, maxlen) == NULL) {
ret = -1;
@@ -663,7 +663,7 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char
*value, i
if (ret == 0 && found)
ret = 1;
- return ret ;
+ return ret;
}
/*
@@ -676,7 +676,7 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char
*value, i
int
openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen)
{
- char conf_file[PATH_MAX] ;
+ char conf_file[PATH_MAX];
if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
return -1;
@@ -700,7 +700,7 @@ openvz_copyfile(char* from_path, char* to_path)
return -1;
}
- while(1) {
+ while (1) {
if (openvz_readline(fd, line, sizeof(line)) <= 0)
break;
@@ -739,7 +739,8 @@ openvzCopyDefaultConfig(int vpsid)
char conf_file[PATH_MAX];
int ret = -1;
- if(openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", configfile_value,
PATH_MAX) < 0)
+ if (openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", configfile_value,
+ PATH_MAX) < 0)
goto cleanup;
confdir = openvzLocateConfDir();
@@ -792,10 +793,10 @@ static char
const char *conf_dir_list[] = {"/etc/vz/conf",
"/usr/local/etc/conf", NULL};
int i=0;
- while(conf_dir_list[i]) {
- if(!access(conf_dir_list[i], F_OK))
+ while (conf_dir_list[i]) {
+ if (!access(conf_dir_list[i], F_OK))
return strdup(conf_dir_list[i]);
- i ++;
+ i++;
}
return NULL;
@@ -808,14 +809,13 @@ openvz_readline(int fd, char *ptr, int maxlen)
int n, rc;
char c;
- for(n = 1; n < maxlen; n ++) {
- if( (rc = read(fd, &c, 1)) == 1) {
+ for (n = 1; n < maxlen; n++) {
+ if ( (rc = read(fd, &c, 1)) == 1) {
*ptr++ = c;
- if(c == '\n')
+ if (c == '\n')
break;
- }
- else if(rc == 0) {
- if(n == 1)
+ } else if (rc == 0) {
+ if (n == 1)
return 0; /* EOF condition */
else
break;
@@ -839,20 +839,20 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
int retval = 0;
if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
- return -1;
+ return -1;
fd = open(conf_file, O_RDONLY);
- if(fd == -1)
+ if (fd == -1)
return -1;
- while(1) {
+ while (1) {
ret = openvz_readline(fd, line, sizeof(line));
- if(ret == -1) {
+ if (ret == -1) {
close(fd);
return -1;
}
- if(ret == 0) { /* EoF, UUID was not found */
+ if (ret == 0) { /* EoF, UUID was not found */
uuidstr[0] = 0;
break;
}
@@ -884,7 +884,7 @@ openvzSetDefinedUUID(int vpsid, unsigned char *uuid)
return -1;
if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
- return -1;
+ return -1;
if (openvzGetVPSUUID(vpsid, uuidstr, sizeof(uuidstr)))
return -1;
@@ -892,7 +892,7 @@ openvzSetDefinedUUID(int vpsid, unsigned char *uuid)
if (uuidstr[0] == 0) {
FILE *fp = fopen(conf_file, "a"); /* append */
if (fp == NULL)
- return -1;
+ return -1;
virUUIDFormat(uuid, uuidstr);
@@ -939,16 +939,16 @@ static int openvzAssignUUIDs(void)
return -1;
dp = opendir(conf_dir);
- if(dp == NULL) {
+ if (dp == NULL) {
VIR_FREE(conf_dir);
return 0;
}
- while((dent = readdir(dp))) {
+ while ((dent = readdir(dp))) {
res = sscanf(dent->d_name, "%d.%5s", &vpsid, ext);
- if(!(res == 2 && STREQ(ext, "conf")))
+ if (!(res == 2 && STREQ(ext, "conf")))
continue;
- if(vpsid > 0) /* '0.conf' belongs to the host, ignore it */
+ if (vpsid > 0) /* '0.conf' belongs to the host, ignore it */
openvzSetUUID(vpsid);
}
closedir(dp);
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index d2f91c6..51d05b1 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -88,8 +88,7 @@ struct openvz_driver ovz_driver;
static void cmdExecFree(const char *cmdExec[])
{
int i=-1;
- while(cmdExec[++i])
- {
+ while (cmdExec[++i]) {
VIR_FREE(cmdExec[i]);
}
}
@@ -226,7 +225,7 @@ static int openvzSetInitialConfig(virDomainDefPtr vmdef)
vmdef->fss[0]->type == VIR_DOMAIN_FS_TYPE_MOUNT)
{
- if(virStrToLong_i(vmdef->name, NULL, 10, &vpsid) < 0) {
+ if (virStrToLong_i(vmdef->name, NULL, 10, &vpsid) < 0) {
openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not convert domain name to VEID"));
goto cleanup;
@@ -802,7 +801,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
//TODO: processing NAT and physical device
- if (prog[0] != NULL){
+ if (prog[0] != NULL) {
ADD_ARG_LIT("--save");
if (virRun(prog, NULL) < 0) {
openvzError(VIR_ERR_INTERNAL_ERROR,
@@ -1397,15 +1396,16 @@ static int openvzListDomains(virConnectPtr conn ATTRIBUTE_UNUSED,
ret = virExec(cmd, NULL, NULL,
&pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
- if(ret == -1) {
+ if (ret == -1) {
openvzError(VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZLIST);
return -1;
}
- while(got < nids){
+ while (got < nids) {
ret = openvz_readline(outfd, buf, 32);
- if(!ret) break;
+ if (!ret)
+ break;
if (virStrToLong_i(buf, &endptr, 10, &veid) < 0) {
openvzError(VIR_ERR_INTERNAL_ERROR,
_("Could not parse VPS ID %s"), buf);
@@ -1443,15 +1443,16 @@ static int openvzListDefinedDomains(virConnectPtr conn
ATTRIBUTE_UNUSED,
/* the -S options lists only stopped domains */
ret = virExec(cmd, NULL, NULL,
&pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
- if(ret == -1) {
+ if (ret == -1) {
openvzError(VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZLIST);
return -1;
}
- while(got < nnames){
+ while (got < nnames) {
ret = openvz_readline(outfd, buf, 32);
- if(!ret) break;
+ if (!ret)
+ break;
if (virStrToLong_i(buf, &endptr, 10, &veid) < 0) {
openvzError(VIR_ERR_INTERNAL_ERROR,
_("Could not parse VPS ID %s"), buf);
@@ -1491,7 +1492,7 @@ Version: 2.2
return -1;
/*search line with VEID=vpsid*/
- while(1) {
+ while (1) {
ret = openvz_readline(fd, line, sizeof(line));
if (ret <= 0)
break;
--
1.7.2.2
6:01 p.m.
New subject: [libvirt] [PATCH 6/7] openvz: formatting cleanups
On 09/01/2010 03:24 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> * src/openvz/openvz_conf.c: Whitespace fixes.
> * src/openvz/openvz_driver.c: Likewise.
> ---
>
> Should just be formatting, no content change.
>
ACK.
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:59 p.m.
New subject: [libvirt] [PATCH 7/7] openvz: use virAsprintf to avoid large stacks
* src/openvz/openvz_conf.c (openvzLocateConfFile): Alter
signature.
(openvzGetVPSUUID, openvzSetDefinedUUID)
(openvzWriteVPSConfigParam, openvzReadVPSConfigParam)
(openvzCopyDefaultConfig): Adjust callers.
---
Nuke a few more PATH_MAX stack allocations.
src/openvz/openvz_conf.c | 78 ++++++++++++++++++++++++++++-----------------
1 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 356c7f0..f00f2f4 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -55,7 +55,7 @@
static char *openvzLocateConfDir(void);
static int openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len);
-static int openvzLocateConfFile(int vpsid, char *conffile, int maxlen, const char *ext);
+static int openvzLocateConfFile(int vpsid, char **conffile, const char *ext);
static int openvzAssignUUIDs(void);
int
@@ -615,12 +615,15 @@ error:
int
openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value)
{
- char conf_file[PATH_MAX];
+ char *conf_file;
+ int ret;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
+ if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0)
return -1;
- return openvzWriteConfigParam(conf_file, param, value);
+ ret = openvzWriteConfigParam(conf_file, param, value);
+ VIR_FREE(conf_file);
+ return ret;
}
static int
@@ -676,12 +679,15 @@ openvzReadConfigParam(const char * conf_file ,const char * param,
char *value, i
int
openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen)
{
- char conf_file[PATH_MAX];
+ char *conf_file;
+ int ret;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
+ if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0)
return -1;
- return openvzReadConfigParam(conf_file, param, value, maxlen);
+ ret = openvzReadConfigParam(conf_file, param, value, maxlen);
+ VIR_FREE(conf_file);
+ return ret;
}
static int
@@ -736,7 +742,7 @@ openvzCopyDefaultConfig(int vpsid)
char * confdir = NULL;
char * default_conf_file = NULL;
char configfile_value[PATH_MAX];
- char conf_file[PATH_MAX];
+ char *conf_file = NULL;
int ret = -1;
if (openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", configfile_value,
@@ -747,12 +753,13 @@ openvzCopyDefaultConfig(int vpsid)
if (confdir == NULL)
goto cleanup;
- if (virAsprintf(&default_conf_file, "%s/ve-%s.conf-sample", confdir,
configfile_value) < 0) {
+ if (virAsprintf(&default_conf_file, "%s/ve-%s.conf-sample", confdir,
+ configfile_value) < 0) {
virReportOOMError();
goto cleanup;
}
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
+ if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0)
goto cleanup;
if (openvz_copyfile(default_conf_file, conf_file)<0)
@@ -762,6 +769,7 @@ openvzCopyDefaultConfig(int vpsid)
cleanup:
VIR_FREE(confdir);
VIR_FREE(default_conf_file);
+ VIR_FREE(conf_file);
return ret;
}
@@ -770,7 +778,7 @@ cleanup:
* 0 - OK
*/
static int
-openvzLocateConfFile(int vpsid, char *conffile, int maxlen, const char *ext)
+openvzLocateConfFile(int vpsid, char **conffile, const char *ext)
{
char * confdir;
int ret = 0;
@@ -779,9 +787,11 @@ openvzLocateConfFile(int vpsid, char *conffile, int maxlen, const
char *ext)
if (confdir == NULL)
return -1;
- if (snprintf(conffile, maxlen, "%s/%d.%s",
- confdir, vpsid, ext ? ext : "conf") >= maxlen)
+ if (virAsprintf(conffile, "%s/%d.%s", confdir, vpsid,
+ ext ? ext : "conf") < 0) {
+ virReportOOMError();
ret = -1;
+ }
VIR_FREE(confdir);
return ret;
@@ -830,27 +840,25 @@ openvz_readline(int fd, char *ptr, int maxlen)
static int
openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
{
- char conf_file[PATH_MAX];
+ char *conf_file;
char line[1024];
char *saveptr = NULL;
char *uuidbuf;
char *iden;
int fd, ret;
- int retval = 0;
+ int retval = -1;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
+ if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0)
return -1;
fd = open(conf_file, O_RDONLY);
if (fd == -1)
- return -1;
+ goto cleanup;
while (1) {
ret = openvz_readline(fd, line, sizeof(line));
- if (ret == -1) {
- close(fd);
- return -1;
- }
+ if (ret == -1)
+ goto cleanup;
if (ret == 0) { /* EoF, UUID was not found */
uuidstr[0] = 0;
@@ -861,12 +869,18 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
uuidbuf = strtok_r(NULL, "\n", &saveptr);
if (iden != NULL && uuidbuf != NULL && STREQ(iden,
"#UUID:")) {
- if (virStrcpy(uuidstr, uuidbuf, len) == NULL)
- retval = -1;
+ if (virStrcpy(uuidstr, uuidbuf, len) == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
break;
}
}
- close(fd);
+ retval = 0;
+cleanup:
+ if (0 <= fd)
+ close(fd);
+ VIR_FREE(conf_file);
return retval;
}
@@ -877,22 +891,23 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
int
openvzSetDefinedUUID(int vpsid, unsigned char *uuid)
{
- char conf_file[PATH_MAX];
+ char *conf_file;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ int ret = -1;
if (uuid == NULL)
return -1;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0)
+ if (openvzLocateConfFile(vpsid, &conf_file, "conf") < 0)
return -1;
if (openvzGetVPSUUID(vpsid, uuidstr, sizeof(uuidstr)))
- return -1;
+ goto cleanup;
if (uuidstr[0] == 0) {
FILE *fp = fopen(conf_file, "a"); /* append */
if (fp == NULL)
- return -1;
+ goto cleanup;
virUUIDFormat(uuid, uuidstr);
@@ -900,10 +915,13 @@ openvzSetDefinedUUID(int vpsid, unsigned char *uuid)
and be careful always to close the stream. */
if ((fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0)
+ (fclose(fp) == EOF))
- return -1;
+ goto cleanup;
}
- return 0;
+ ret = 0;
+cleanup:
+ VIR_FREE(conf_file);
+ return ret;
}
static int
--
1.7.2.2
4:41 p.m.
New subject: [libvirt] [PATCH 7/7] openvz: use virAsprintf to avoid large stacks
2010/9/1 Eric Blake <eblake(a)redhat.com>:
* src/openvz/openvz_conf.c (openvzLocateConfFile): Alter
signature.
(openvzGetVPSUUID, openvzSetDefinedUUID)
(openvzWriteVPSConfigParam, openvzReadVPSConfigParam)
(openvzCopyDefaultConfig): Adjust callers.
---
Nuke a few more PATH_MAX stack allocations.
src/openvz/openvz_conf.c | 78 ++++++++++++++++++++++++++++-----------------
1 files changed, 48 insertions(+), 30 deletions(-)
@@ -861,12 +869,18 @@ openvzGetVPSUUID(int vpsid, char *uuidstr,
size_t len)
uuidbuf = strtok_r(NULL, "\n", &saveptr);
if (iden != NULL && uuidbuf != NULL && STREQ(iden,
"#UUID:")) {
- if (virStrcpy(uuidstr, uuidbuf, len) == NULL)
- retval = -1;
+ if (virStrcpy(uuidstr, uuidbuf, len) == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
virStrcpy cannot fail because of OOM, it doesn't do an allocation.
When it returns NULL, this means that one tried to copy too much data
to the given destination buffer. The typical error message in such a
case looks like this:
if (virStrcpy(sa_qemu.sun_path, unixfile,
sizeof(sa_qemu.sun_path)) == NULL) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("Unix socket '%s' too big for destination"),
unixfile);
goto cleanup;
}
break;
}
}
- close(fd);
+ retval = 0;
+cleanup:
+ if (0 <= fd)
if (fd >= 0) reads nicer here.
+ close(fd);
+ VIR_FREE(conf_file);
return retval;
}
ACK, with these two comments addressed.
Matthias
6:01 p.m.
New subject: [libvirt] [PATCH 7/7] openvz: use virAsprintf to avoid large stacks
On 09/01/2010 03:41 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)redhat.com>:
> * src/openvz/openvz_conf.c (openvzLocateConfFile): Alter
> signature.
> (openvzGetVPSUUID, openvzSetDefinedUUID)
> (openvzWriteVPSConfigParam, openvzReadVPSConfigParam)
> (openvzCopyDefaultConfig): Adjust callers.
> ---
>
> Nuke a few more PATH_MAX stack allocations.
>
> src/openvz/openvz_conf.c | 78 ++++++++++++++++++++++++++++-----------------
> 1 files changed, 48 insertions(+), 30 deletions(-)
>
> @@ -861,12 +869,18 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
> uuidbuf = strtok_r(NULL, "\n",&saveptr);
>
> if (iden != NULL&& uuidbuf != NULL&& STREQ(iden,
"#UUID:")) {
> - if (virStrcpy(uuidstr, uuidbuf, len) == NULL)
> - retval = -1;
> + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) {
> + virReportOOMError();
> + goto cleanup;
> + }
virStrcpy cannot fail because of OOM, it doesn't do an allocation.
/me slaps forehead. Of course not; but at the same time, silently
returning -1 is wrong.
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("Unix socket '%s' too big for
destination"),
unixfile);
Unrelated, but the example you copied from reads a bit awkwardly (maybe
"Unix socket too small to contain '%s'" would have sounded better?), but
that's a separate patch.
> break;
> }
> }
> - close(fd);
> + retval = 0;
> +cleanup:
> + if (0<= fd)
if (fd>= 0) reads nicer here.
That's coreutils HACKING policy rearing it's head (where Jim always
prefers < over >); I've adjusted accordingly.
ACK, with these two comments addressed.
Thanks for the review; I squashed in the diff below, then pushed.
That leaves 2, 3, and 5 of the series still awaiting review.
diff --git i/src/openvz/openvz_conf.c w/src/openvz/openvz_conf.c
index f00f2f4..ec11bbc 100644
--- i/src/openvz/openvz_conf.c
+++ w/src/openvz/openvz_conf.c
@@ -870,7 +870,8 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
if (iden != NULL && uuidbuf != NULL && STREQ(iden,
"#UUID:")) {
if (virStrcpy(uuidstr, uuidbuf, len) == NULL) {
- virReportOOMError();
+ openvzError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid uuid %s"), uuidbuf);
goto cleanup;
}
break;
@@ -878,7 +879,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
}
retval = 0;
cleanup:
- if (0 <= fd)
+ if (fd >= 0)
close(fd);
VIR_FREE(conf_file);
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5198
days inactive
5199
days old
21 comments
2 participants
participants (2)
-
Eric Blake -
Matthias Bolte