[libvirt] [PATCH 1/3] virshDomainNameCompleter: Prune accepted flags

Michal Privoznik mprivozn at redhat.com
Wed Jan 31 14:17:40 UTC 2018

On 01/31/2018 02:39 PM, Peter Krempa wrote:
> On Wed, Jan 31, 2018 at 14:34:24 +0100, Michal Privoznik wrote:
>> On 01/31/2018 01:09 PM, Peter Krempa wrote:
>>> On Thu, Jan 25, 2018 at 15:22:18 +0100, Michal Privoznik wrote:
>>>> Only a small subset of VIR_CONNECT_LIST_DOMAINS_* flags are
>>>> actually used in virsh code. Remove the unused ones.
>>> I don't think this is well justified. All of the flags are implemented
>>> in virsh code in general.
>>> I see almost all of them implemented in the cmdList command.
>> We don't have to care about cmdList since it does not use
>> virshDomainNameCompleter. What this patch addresses is use of
>> VIR_CONNECT_LIST_DOMAINS_* for this specific completer callback. And our
>> commands accept domains only from a small subset of those. It's safe to
>> say that 99% of commands take active/inactive domains, and the rest 1%
>> is more specific. Anyway, lets try to print out all used flags for this
>> completer:
> All this information is not present in the commit message. You
> generalize that the "flags are (not) actually used in virsh code", which
> is simply false. The commit message does in no way mention that you are
> specifically talking about usage of the flags in the completion code,
> which leads to false assumptions.

Ah sorry, I though that "virsh*Completer: " prefix was clear enough. It
isn't. Okay, I'll update the commit message before pushing.

>> libvirt.git $ git grep VIRSH_COMMON_OPT_DOMAIN tools/ | cut -d'(' -f 2 |
>> cut -d')' -f 1 | sort -u
>>> You either need to rewrite the commit message or the patch below is
>>> flawed.
>> I guess you just misunderstood this patch.
> You did not explain it well enough in the commit message.


More information about the libvir-list mailing list