On 06.12.2012 13:29, Laine Stump wrote:
> On 12/06/2012 06:34 AM, Michal Privoznik wrote:
>> If the debugging is enabled, the virCommand subsystem catches debug
>> messages in the command output as well. In that case, we can't assume
>> the string corresponding to command's stdout will start with specific
>> prefix. But the prefix can be moved deeper in the string. This bug
>> shows itself when parsing dnsmasq output:
>>
>> 2012-12-06 11:18:11.445+0000: 18491: error :
>> dnsmasqCapsSetFromBuffer:664 : internal error cannot parse
>> /usr/sbin/dnsmasq version number in '2012-12-06 11:11:02.232+0000:
>> 18492: debug : virFileClose:72 : Closed fd 22'
>>
>> We can clearly see that the output of dnsmasq --version
>> doesn't start with expected "Dnsmasq version " string but a libvirt
>> debug output.
>
> This is a bug in virCommand, and should also affect
> qemuCapsParseHelpStr(). Is there no way to fix it at the source, rather
> than working around it like this?
Oh, right. The reason why this doesn't get exposed with
qemuCapsParseHelpStr() is - we are not keeping stderr when executing
'qemu --help' but we are doing that for dnsmasq. So I guess this patch
is obsolete and should be instead replaced by:
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
index 4f210d2..bee3b61 100644
--- a/src/util/dnsmasq.c
+++ b/src/util/dnsmasq.c
@@ -715,7 +715,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool
force)
cmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
virCommandSetOutputBuffer(cmd, &version);
- virCommandSetErrorBuffer(cmd, &version);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL) < 0) {
@@ -727,7 +726,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool
force)
cmd = virCommandNewArgList(caps->binaryPath, "--help", NULL);
virCommandSetOutputBuffer(cmd, &help);
- virCommandSetErrorBuffer(cmd, &help);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL) < 0) {
>
>
>> ---
>> src/util/dnsmasq.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
>> index 4f210d2..de0293a 100644
>> --- a/src/util/dnsmasq.c
>> +++ b/src/util/dnsmasq.c
>> @@ -641,9 +641,9 @@ dnsmasqCapsSetFromBuffer(dnsmasqCapsPtr caps, const char *buf)
>>
>> caps->noRefresh = true;
>>
>> - p = STRSKIP(buf, DNSMASQ_VERSION_STR);
>> - if (!p)
>> + if (!(p = strstr(buf, DNSMASQ_VERSION_STR)))
>> goto fail;
>> + p += sizeof(DNSMASQ_VERSION_STR) - 1;
>
> Why -1 ? That happens to work, but wouldn't if the version string didn't
> end with a space.
sizeof(const char[]) counts '\0' at the end. We don't want that here.
>
>> virSkipSpaces(&p);
>> if (virParseVersionString(p, &caps->version, true) < 0)
>> goto fail;
>
> I'd rather make sure that nobody has a bright idea on how to fix
> virCommand before pushing this.
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list