[libvirt] [PATCH] qemu: use default machine type if missing it in qemu command line

BZ:https://bugzilla.redhat.com/show_bug.cgi?id=871273 when using virsh qemu-attach to attach an existing qemu process, if it misses the -M option in qemu command line, libvirtd crashed because the NULL value of def->os.machine in later use. Example: /usr/libexec/qemu-kvm -name foo \ -cdrom /var/lib/libvirt/images/boot.img \ -monitor unix:/tmp/demo,server,nowait \ error: End of file while reading data: Input/output error error: Failed to reconnect to the hypervisor This patch tries to set default machine type if the value of def->os.machine is still NULL after qemu command line parsing. --- src/qemu/qemu_command.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe99f5d..0b6d2f8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8768,6 +8768,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } } + if (!def->os.machine) { + const char *defaultMachine = + virCapabilitiesDefaultGuestMachine(caps, + def->os.type, + def->os.arch, + virDomainVirtTypeToString(def->virtType)); + if (defaultMachine != NULL) + if (!(def->os.machine = strdup(defaultMachine))) + goto no_memory; + } + if (!nographics && def->ngraphics == 0) { virDomainGraphicsDefPtr sdl; const char *display = qemuFindEnv(progenv, "DISPLAY"); -- 1.7.1

BZ:https://bugzilla.redhat.com/show_bug.cgi?id=871273 when using virsh qemu-attach to attach an existing qemu process, if it misses the -M option in qemu command line, libvirtd crashed because the NULL value of def->os.machine in later use. Example: /usr/libexec/qemu-kvm -name foo \ -cdrom /var/lib/libvirt/images/boot.img \ -monitor unix:/tmp/demo,server,nowait \ error: End of file while reading data: Input/output error error: Failed to reconnect to the hypervisor This patch tries to set default machine type if the value of def->os.machine is still NULL after qemu command line parsing. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_command.c | 11 +++++++++++ src/qemu/qemu_driver.c | 7 +++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 271273c..3be1e1f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1845,10 +1845,12 @@ no_memory: const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps, const char *name) - { size_t i; + if (!name) + return NULL; + for (i = 0 ; i < caps->nmachineTypes ; i++) { if (!caps->machineAliases[i]) continue; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe99f5d..0b6d2f8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8768,6 +8768,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } } + if (!def->os.machine) { + const char *defaultMachine = + virCapabilitiesDefaultGuestMachine(caps, + def->os.type, + def->os.arch, + virDomainVirtTypeToString(def->virtType)); + if (defaultMachine != NULL) + if (!(def->os.machine = strdup(defaultMachine))) + goto no_memory; + } + if (!nographics && def->ngraphics == 0) { virDomainGraphicsDefPtr sdl; const char *display = qemuFindEnv(progenv, "DISPLAY"); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 267bbf1..15f4211 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1517,9 +1517,12 @@ static int qemudNumDomains(virConnectPtr conn) { static int qemuCanonicalizeMachine(virDomainDefPtr def, qemuCapsPtr caps) { - const char *canon = qemuCapsGetCanonicalMachine(caps, def->os.machine); + const char *canon; - if (canon != def->os.machine) { + if (!(canon = qemuCapsGetCanonicalMachine(caps, def->os.machine))) + return 0; + + if (STRNEQ(canon, def->os.machine)) { char *tmp; if (!(tmp = strdup(canon))) { virReportOOMError(); -- 1.7.1

On 11/01/2012 05:46 AM, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=871273 when using virsh qemu-attach to attach an existing qemu process, if it misses the -M option in qemu command line, libvirtd crashed because the NULL value of def->os.machine in later use.
Example: /usr/libexec/qemu-kvm -name foo \ -cdrom /var/lib/libvirt/images/boot.img \ -monitor unix:/tmp/demo,server,nowait \
error: End of file while reading data: Input/output error error: Failed to reconnect to the hypervisor
This patch tries to set default machine type if the value of def->os.machine is still NULL after qemu command line parsing. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_command.c | 11 +++++++++++ src/qemu/qemu_driver.c | 7 +++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
ACK. This fixes the problems I saw in v1 - def->os.machine can still be NULL (although less likely in the normal case of learning the default machine type), but now we don't dereference the NULL. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/01/2012 11:01 PM, Eric Blake wrote:
On 11/01/2012 05:46 AM, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=871273 when using virsh qemu-attach to attach an existing qemu process, if it misses the -M option in qemu command line, libvirtd crashed because the NULL value of def->os.machine in later use.
Example: /usr/libexec/qemu-kvm -name foo \ -cdrom /var/lib/libvirt/images/boot.img \ -monitor unix:/tmp/demo,server,nowait \
error: End of file while reading data: Input/output error error: Failed to reconnect to the hypervisor
This patch tries to set default machine type if the value of def->os.machine is still NULL after qemu command line parsing. --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_command.c | 11 +++++++++++ src/qemu/qemu_driver.c | 7 +++++-- 3 files changed, 19 insertions(+), 3 deletions(-) ACK. This fixes the problems I saw in v1 - def->os.machine can still be NULL (although less likely in the normal case of learning the default machine type), but now we don't dereference the NULL.
Thanks and pushed. Guannnan

On 11/01/2012 04:36 AM, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=871273 when using virsh qemu-attach to attach an existing qemu process, if it misses the -M option in qemu command line, libvirtd crashed because the NULL value of def->os.machine in later use.
Example: /usr/libexec/qemu-kvm -name foo \ -cdrom /var/lib/libvirt/images/boot.img \ -monitor unix:/tmp/demo,server,nowait \
error: End of file while reading data: Input/output error error: Failed to reconnect to the hypervisor
This patch tries to set default machine type if the value of def->os.machine is still NULL after qemu command line parsing. --- src/qemu/qemu_command.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe99f5d..0b6d2f8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8768,6 +8768,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } }
+ if (!def->os.machine) { + const char *defaultMachine = + virCapabilitiesDefaultGuestMachine(caps, + def->os.type, + def->os.arch, + virDomainVirtTypeToString(def->virtType)); + if (defaultMachine != NULL) + if (!(def->os.machine = strdup(defaultMachine))) + goto no_memory;
This code still leaves the possibility of def->os.machine being NULL (such as if defaultMachine is NULL). Shouldn't you error out in that case? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Guannan Ren