Re: [libvirt] [PATCH 3/4] parallels: fix parallelsDomainDefineXML for existing containers
On Mon, Sep 10, 2012 at 07:22:44PM +0400, Dmitry Guryanov wrote:
Fix code, which checks what is changed in virDomainDef structure.
It looks slightly different for containers and VMs: containers haven't
boot devices, but have init path
Signed-off-by: Dmitry Guryanov <dguryanov(a)parallels.com>
---
src/parallels/parallels_driver.c | 42 ++++++++++++++++++++++++++++---------
1 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index ace75a6..fd6ba88 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1484,24 +1484,46 @@ parallelsApplyChanges(virDomainObjPtr dom, virDomainDefPtr new)
return -1;
}
- /* we fill only type and arch fields in parallelsLoadDomain, so
- * we can check that all other paramenters are null */
+ /* we fill only type and arch fields in parallelsLoadDomain for
+ * hvm type and also init for containers, so we can check that all
+ * other paramenters are null and boot devices config is default */
+
if (!STREQ_NULLABLE(old->os.type, new->os.type) ||
!STREQ_NULLABLE(old->os.arch, new->os.arch) ||
So here you implicitely allow an update where the new os.type or
os.arch would be NULL, I assume that any definition in the system
should have those set, right ?
- new->os.machine != NULL || new->os.nBootDevs != 1 ||
- new->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK ||
- new->os.bootmenu != 0 || new->os.init != NULL ||
- new->os.initargv != NULL || new->os.kernel != NULL ||
- new->os.initrd != NULL || new->os.cmdline != NULL ||
- new->os.root != NULL || new->os.loader != NULL ||
- new->os.bootloader != NULL || new->os.bootloaderArgs != NULL ||
- new->os.smbios_mode != 0 || new->os.bios.useserial != 0) {
+ new->os.machine != NULL || new->os.bootmenu != 0 ||
+ new->os.kernel != NULL || new->os.initrd != NULL ||
+ new->os.cmdline != NULL || new->os.root != NULL ||
+ new->os.loader != NULL || new->os.bootloader != NULL ||
+ new->os.bootloaderArgs != NULL || new->os.smbios_mode != 0 ||
+ new->os.bios.useserial != 0) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("changing OS parameters is not supported "
"by parallels driver"));
return -1;
}
+ if (STREQ(new->os.type, "hvm")) {
But here you cmpare to new->os.type, which could potentially be NULL
at this point. Shouldn't that be changed to old->os.type instead ?
+ if (new->os.nBootDevs != 1 ||
+ new->os.bootDevs[0] != VIR_DOMAIN_BOOT_DISK ||
+ new->os.init != NULL || new->os.initargv != NULL) {
+
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("changing OS parameters is not supported "
+ "by parallels driver"));
+ return -1;
+ }
indentation error, here we are closing the second "if"
+ } else {
+ if (new->os.nBootDevs != 0 ||
+ !STREQ_NULLABLE(old->os.init, new->os.init) ||
+ (new->os.initargv != NULL && new->os.initargv[0] != NULL)) {
+
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("changing OS parameters is not supported "
+ "by parallels driver"));
+ return -1;
+ }
+ }
+
if (!STREQ_NULLABLE(old->emulator, new->emulator)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
Postponed with the indentation fix waiting for the 4th patch to be
resubmitted,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/