7:33 a.m.
Signed-off-by: Tal Kain <tal.kain(a)ravellosystems.com>
---
src/qemu/qemu_capabilities.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 50712b0..b235059 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -860,8 +860,8 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
{
virCapsPtr caps;
int i;
-
- if ((caps = virCapabilitiesNew(virArchFromHost(),
+ virArch hostarch = virArchFromHost();
+ if ((caps = virCapabilitiesNew(hostarch,
1, 1)) == NULL)
goto error;
@@ -874,7 +874,7 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
VIR_WARN("Failed to query host NUMA topology, disabling NUMA
capabilities");
}
- if (virQEMUCapsInitCPU(caps, virArchFromHost()) < 0)
+ if (virQEMUCapsInitCPU(caps, hostarch) < 0)
VIR_WARN("Failed to get host CPU");
/* Add the power management features of the host */
@@ -891,7 +891,7 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
*/
for (i = 0 ; i < VIR_ARCH_LAST ; i++)
if (virQEMUCapsInitGuest(caps, cache,
- virArchFromHost(),
+ hostarch,
i) < 0)
goto error;
@@ -1639,13 +1639,14 @@ int virQEMUCapsGetDefaultVersion(virCapsPtr caps,
if (*version > 0)
return 0;
+ virArch hostarch = virArchFromHost();
if ((binary = virCapabilitiesDefaultGuestEmulator(caps,
"hvm",
- virArchFromHost(),
+ hostarch,
"qemu")) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot find suitable emulator for %s"),
- virArchToString(virArchFromHost()));
+ virArchToString(hostarch));
return -1;
}
--
1.7.9.5
8:32 a.m.
On Mon, Apr 08, 2013 at 03:33:07PM +0300, Tal Kain wrote:
Signed-off-by: Tal Kain <tal.kain(a)ravellosystems.com>
---
src/qemu/qemu_capabilities.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
What is the motivation for doing this ? it just looks like
overkill to me, since virArchFromHost isn't really an
expensive method to invoke.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:38 a.m.
On Mon, Apr 8, 2013 at 4:32 PM, Daniel P. Berrange <berrange(a)redhat.com>wrote:
On Mon, Apr 08, 2013 at 03:33:07PM +0300, Tal Kain wrote:
> Signed-off-by: Tal Kain <tal.kain(a)ravellosystems.com>
> ---
> src/qemu/qemu_capabilities.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
What is the motivation for doing this ? it just looks like
overkill to me, since virArchFromHost isn't really an
expensive method to invoke.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/:|
|: http://libvirt.org -o- http://virt-manager.org:|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/:|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc:|
Hey Daniel,
Thank you for the quick reply.
This patch's motivation is not about making this function faster.
Even if virArchFromHost isn't expensive, It seemed to me like there is no
good reason for calling it 3 times at the same context, It is just
unnecessary.
Why using a local variable instead of calling the same function three times
is an overkill?
From my perspective, by calling it just once I'm making it easier
for a
reader to understand the usage of this function without reading its code.
Thanks in advance,
Tal Kain.
5:04 p.m.
On 04/09/2013 12:38 AM, Tal Kain wrote:
On Mon, Apr 8, 2013 at 4:32 PM, Daniel P. Berrange
<berrange(a)redhat.com>wrote:
> On Mon, Apr 08, 2013 at 03:33:07PM +0300, Tal Kain wrote:
>> Signed-off-by: Tal Kain <tal.kain(a)ravellosystems.com>
>> ---
>> src/qemu/qemu_capabilities.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> What is the motivation for doing this ? it just looks like
> overkill to me, since virArchFromHost isn't really an
> expensive method to invoke.
>
This patch's motivation is not about making this function
faster.
Even if virArchFromHost isn't expensive, It seemed to me like there is no
good reason for calling it 3 times at the same context, It is just
unnecessary.
Why using a local variable instead of calling the same function three times
is an overkill?
Personally, I don't think the patch is too bad, and I'll probably apply
it unless Dan can give a stronger objection.
>From my perspective, by calling it just once I'm making it easier for a
reader to understand the usage of this function without reading its code.
A good compiler will already factor out invariant calls; but we don't
use enough annotations to tell the compiler that virArchFromHost is
invariant.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:36 a.m.
On Wed, Apr 10, 2013 at 1:04 AM, Eric Blake <eblake(a)redhat.com> wrote:
On 04/09/2013 12:38 AM, Tal Kain wrote:
> On Mon, Apr 8, 2013 at 4:32 PM, Daniel P. Berrange <berrange(a)redhat.com
>wrote:
>
>> On Mon, Apr 08, 2013 at 03:33:07PM +0300, Tal Kain wrote:
>>> Signed-off-by: Tal Kain <tal.kain(a)ravellosystems.com>
>>> ---
>>> src/qemu/qemu_capabilities.c | 13 +++++++------
>>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> What is the motivation for doing this ? it just looks like
>> overkill to me, since virArchFromHost isn't really an
>> expensive method to invoke.
>>
> This patch's motivation is not about making this function faster.
> Even if virArchFromHost isn't expensive, It seemed to me like there is no
> good reason for calling it 3 times at the same context, It is just
> unnecessary.
> Why using a local variable instead of calling the same function three
times
> is an overkill?
Personally, I don't think the patch is too bad, and I'll probably apply
it unless Dan can give a stronger objection.
>
>>From my perspective, by calling it just once I'm making it easier for a
> reader to understand the usage of this function without reading its code.
A good compiler will already factor out invariant calls; but we don't
use enough annotations to tell the compiler that virArchFromHost is
invariant.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Hey,
I was wondering if there are any updates about this patch
Thanks in advance,
Tal Kain
7:42 a.m.
On 04/18/2013 01:36 AM, Tal Kain wrote:
>
> Personally, I don't think the patch is too bad, and I'll probably apply
> it unless Dan can give a stronger objection.
>
I was wondering if there are any updates about this patch
Sure, I just applied it. I shortened the subject line (generally commit
messages should be < ~60 columns, so that 'git log --oneline -30'
doesn't cause line wrapping). I also tweaked the code to match our
usual conventions (but not absolute rule) of C89 declaration before
statement.
qemu: simplify use of virArchFromHost
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4236
days inactive
4246
days old
5 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Tal Kain