[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virsh: fixed domdisplay command



On 07/23/2012 09:42 PM, Eric Blake wrote:
> On 07/23/2012 12:51 PM, Martin Kletzander wrote:
>> The 'domdisplay' command didn't properly evaluate '--include-password'
>> option.
>> ---
>>  tools/virsh.c |   35 +++++++++++++++++++++++------------
>>  1 files changed, 23 insertions(+), 12 deletions(-)
> 
> In addition to Doug's review...
> 
>> -    doc = virDomainGetXMLDesc(dom, 0);
>> +    if (!vshCommandOptBool(cmd, "include-password"))
>> +        doc = virDomainGetXMLDesc(dom, 0);
>> +    else {
>> +        if (ctl->conn->flags & VIR_DOMAIN_XML_SECURE) {
>> +            vshError(ctl, _("Cannot get password with read-only connection"));
>> +            goto cleanup;
>> +        }
> 
> We shouldn't have to do this filtering here.  Just attempt the access
> always (when the options say to); it (better) fail at the driver level
> if the connection was read-only, for less work here in virsh, and so
> that we can actually validate that the security checking is being done
> at the driver level.
> 

I see now the patch is all wrong, the check was supposed to be
(ctl->conn->flags & VIR_CONNECT_RO) && VIR_DOMAIN_XML_SECURE
anyway.

I can omit the check, it will just result in different error message for
the user, no problem with that.

Sending a v2 (I never thought there could be this many errors in such a
small patch).

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]