11:51 a.m.
The patch below fixes capabilities xml generation for
the qemu driver if the emulators aren't found in the
hardcoded paths. Current behavior will add a <guest>
entry for the emulator even if it does not exist, patch
fixes this to check that we actually have access.
This brings up another issue, that hardcoded paths
aren't exactly distro friendly. I'm not really familiar
with what options we have to allow specifying these
at build time (or something else). Anyone have an
idea?
Thanks,
Cole
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index dc9e42a..0328cc1 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
virCapsGuestPtr guest;
int i;
+ /* Check for existance of base emulator */
+ if (access(info->binary, X_OK) == -1)
+ return 0;
+
if ((guest = virCapabilitiesAddGuest(caps,
hvm ? "hvm" : "xen",
info->arch,
@@ -241,9 +245,7 @@ qemudCapsInitGuest(virCapsPtr caps,
return -1;
if (hvm) {
- /* Check for existance of base emulator */
- if (access(info->binary, X_OK) == 0 &&
- virCapabilitiesAddGuestDomain(guest,
+ if (virCapabilitiesAddGuestDomain(guest,
"qemu",
NULL,
NULL,
@@ -263,6 +265,7 @@ qemudCapsInitGuest(virCapsPtr caps,
return -1;
if (access("/dev/kvm", F_OK) == 0 &&
+ access("/usr/bin/qemu-kvm", X_OK) == 0 &&
virCapabilitiesAddGuestDomain(guest,
"kvm",
"/usr/bin/qemu-kvm",
12:16 p.m.
On Wed, Aug 20, 2008 at 12:51:05PM -0400, Cole Robinson wrote:
The patch below fixes capabilities xml generation for
the qemu driver if the emulators aren't found in the
hardcoded paths. Current behavior will add a <guest>
entry for the emulator even if it does not exist, patch
fixes this to check that we actually have access.
This brings up another issue, that hardcoded paths
aren't exactly distro friendly. I'm not really familiar
with what options we have to allow specifying these
at build time (or something else). Anyone have an
idea?
Ideally we'd just list the binary names, and search in $PATH for them.
Patches for this welcome...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index dc9e42a..0328cc1 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
virCapsGuestPtr guest;
int i;
+ /* Check for existance of base emulator */
+ if (access(info->binary, X_OK) == -1)
+ return 0;
+
This isn't right - this means that if KVM is installed, but QEMU is
not installed you won't get any capabilities.
Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
up-front. And then generated the capabilites if either QEMU or KVM is
available.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:56 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Wed, Aug 20, 2008 at 12:51:05PM -0400, Cole Robinson wrote:
> The patch below fixes capabilities xml generation for
> the qemu driver if the emulators aren't found in the
> hardcoded paths. Current behavior will add a <guest>
> entry for the emulator even if it does not exist, patch
> fixes this to check that we actually have access.
>
> This brings up another issue, that hardcoded paths
> aren't exactly distro friendly. I'm not really familiar
> with what options we have to allow specifying these
> at build time (or something else). Anyone have an
> idea?
Ideally we'd just list the binary names, and search in $PATH for them.
Patches for this welcome...
The gnulib "findprog" module can be used to do this, but
there are two obstacles:
- it's GPL (probably just because...
- it exits on an out of memory error
However, it's not a big deal to rewrite it not to exit,
and make dependent modules will be LGPLv2+. Will require
pulling the mfile_name_cat function out into its own module.
I'm working on this.
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index dc9e42a..0328cc1 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
> virCapsGuestPtr guest;
> int i;
>
> + /* Check for existance of base emulator */
> + if (access(info->binary, X_OK) == -1)
> + return 0;
> +
This isn't right - this means that if KVM is installed, but QEMU is
not installed you won't get any capabilities.
Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
up-front. And then generated the capabilites if either QEMU or KVM is
available.
Daniel
12:16 p.m.
Daniel P. Berrange wrote:
On Wed, Aug 20, 2008 at 12:51:05PM -0400, Cole Robinson wrote:
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index dc9e42a..0328cc1 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
> virCapsGuestPtr guest;
> int i;
>
> + /* Check for existance of base emulator */
> + if (access(info->binary, X_OK) == -1)
> + return 0;
> +
This isn't right - this means that if KVM is installed, but QEMU is
not installed you won't get any capabilities.
Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
up-front. And then generated the capabilites if either QEMU or KVM is
available.
Okay, I think this patch solves the issues. We check upfront
for the base emulator and potential kvm emulators (qemu-kvm,
and /usr/bin/kvm for ubuntu/upstream .spec). If nothing is
found, just return. If only the base emulator is found, skip
kvm even if /dev/kvm exists. If only kvm bin is found, add
the base emulator capabilities only if emulator and host arch
matches.
Thanks,
Cole
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ee5851d..4ef75dd 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -226,7 +226,29 @@ qemudCapsInitGuest(virCapsPtr caps,
const struct qemu_arch_info *info,
int hvm) {
virCapsGuestPtr guest;
- int i;
+ int i, haskvm, hasbase, samearch;
+ const char *kvmbin = NULL;
+
+ /* Check for existance of base emulator */
+ hasbase = (access(info->binary, X_OK) == 0);
+
+ samearch = STREQ(info->arch, hostmachine);
+ if (samearch) {
+ const char *const kvmbins[] = { "/usr/bin/qemu-kvm", /* Fedora */
+ "/usr/bin/kvm" }; /* Upstream .spec */
+
+ for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
+ if ((haskvm = (access(kvmbins[i], X_OK) == 0))) {
+ kvmbin = kvmbins[i];
+ break;
+ }
+ }
+ } else {
+ haskvm = 0;
+ }
+
+ if (!hasbase && !haskvm)
+ return 0;
if ((guest = virCapabilitiesAddGuest(caps,
hvm ? "hvm" : "xen",
@@ -239,8 +261,7 @@ qemudCapsInitGuest(virCapsPtr caps,
return -1;
if (hvm) {
- /* Check for existance of base emulator */
- if (access(info->binary, X_OK) == 0 &&
+ if (hasbase &&
virCapabilitiesAddGuestDomain(guest,
"qemu",
NULL,
@@ -250,7 +271,7 @@ qemudCapsInitGuest(virCapsPtr caps,
return -1;
/* If guest & host match, then we can accelerate */
- if (STREQ(info->arch, hostmachine)) {
+ if (samearch) {
if (access("/dev/kqemu", F_OK) == 0 &&
virCapabilitiesAddGuestDomain(guest,
"kqemu",
@@ -261,9 +282,10 @@ qemudCapsInitGuest(virCapsPtr caps,
return -1;
if (access("/dev/kvm", F_OK) == 0 &&
+ haskvm &&
virCapabilitiesAddGuestDomain(guest,
"kvm",
- "/usr/bin/qemu-kvm",
+ kvmbin,
NULL,
0,
NULL) == NULL)
5:52 a.m.
On Mon, Aug 25, 2008 at 01:16:39PM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
> On Wed, Aug 20, 2008 at 12:51:05PM -0400, Cole Robinson wrote:
>
>> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
>> index dc9e42a..0328cc1 100644
>> --- a/src/qemu_conf.c
>> +++ b/src/qemu_conf.c
>> @@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
>> virCapsGuestPtr guest;
>> int i;
>>
>> + /* Check for existance of base emulator */
>> + if (access(info->binary, X_OK) == -1)
>> + return 0;
>> +
>
> This isn't right - this means that if KVM is installed, but QEMU is
> not installed you won't get any capabilities.
>
> Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
> up-front. And then generated the capabilites if either QEMU or KVM is
> available.
>
Okay, I think this patch solves the issues. We check upfront
for the base emulator and potential kvm emulators (qemu-kvm,
and /usr/bin/kvm for ubuntu/upstream .spec). If nothing is
found, just return. If only the base emulator is found, skip
kvm even if /dev/kvm exists. If only kvm bin is found, add
the base emulator capabilities only if emulator and host arch
matches.
ACK, this looks good now.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10 a.m.
On Mon, Aug 25, 2008 at 01:16:39PM -0400, Cole Robinson wrote:
Daniel P. Berrange wrote:
> This isn't right - this means that if KVM is installed, but QEMU is
> not installed you won't get any capabilities.
>
> Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
> up-front. And then generated the capabilites if either QEMU or KVM is
> available.
>
Okay, I think this patch solves the issues. We check upfront
for the base emulator and potential kvm emulators (qemu-kvm,
and /usr/bin/kvm for ubuntu/upstream .spec). If nothing is
found, just return. If only the base emulator is found, skip
kvm even if /dev/kvm exists. If only kvm bin is found, add
the base emulator capabilities only if emulator and host arch
matches.
That looks fine, applied and commited to CVS,
thanks !
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/
5926
days inactive
5939
days old
5 comments
4 participants
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering