Eric Blake wrote:
> On 04/28/2014 12:42 PM, Jim Fehlig wrote:
>
>> With xend on the way out, installations may not even have
>> /usr/sbin/xend, which results in the following error when the
>> drivers are probed
>>
>> 2014-04-28 18:21:19.271+0000: 22129: error : virCommandWait:2426 :
>> internal error: Child process (/usr/sbin/xend status) unexpected exit
>> status 127: libvirt: error : cannot execute binary /usr/sbin/xend:
>> No such file or directory
>>
>> Check for existence of /usr/sbin/xend before trying to run it with
>> the 'status' option.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
>> ---
>> src/libxl/libxl_driver.c | 19 ++++++++++++-------
>> src/xen/xen_driver.c | 13 ++++++++-----
>> 2 files changed, 20 insertions(+), 12 deletions(-)
>>
>
>
>
>> @@ -256,14 +255,20 @@ libxlDriverShouldLoad(bool privileged)
>> }
>>
>> /* Don't load if legacy xen toolstack (xend) is in use */
>> - cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> - if (virCommandRun(cmd, &status) == 0 && status == 0) {
>> - VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
>> - "libxenlight driver.");
>> + if (virFileExists("/usr/sbin/xend")) {
>> + virCommandPtr cmd;
>> +
>> + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> + if (virCommandRun(cmd, &status) == 0 && status == 0) {
>>
>
> Pre-patch, passing &status to virCommandRun had the effect of avoiding a
> log message if the command ran with non-zero status. But now that we
> are requiring the file to exist, and in particular, since we are already
> using NULL later on...
>
>
>> +++ b/src/xen/xen_driver.c
>> @@ -315,13 +315,16 @@ xenUnifiedProbe(void)
>> static bool
>> xenUnifiedXendProbe(void)
>> {
>> - virCommandPtr cmd;
>> bool ret = false;
>>
>> - cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> - if (virCommandRun(cmd, NULL) == 0)
>> - ret = true;
>> - virCommandFree(cmd);
>> + if (virFileExists("/usr/sbin/xend")) {
>> + virCommandPtr cmd;
>> +
>> + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
>> + if (virCommandRun(cmd, NULL) == 0)
>>
>
> ...here, would it make sense to use virCommandRun(cmd, NULL) instead of
> checking status ourselves?
Yes, I think so. No harm in a log message from virCommandRun on
non-zero exit.
> If the intent is still to avoid a logged
> message when the status is non-zero, a comment in the code might be
> useful (for example, see how virStorageBackendQEMUImgBackingFormat in
> storage_backend.c has a comment).
>
> ACK with either the code simplification or the added comment.
>
Code simplified and pushed. Thanks for the review.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list