Re: [Qemu-devel] [ndctl PATCH 0/8] dimm label space initialization support

[Dan Williams]

On Wed, Oct 19, 2016 at 4:46 PM, Eric Blake <address@hidden> wrote:
> On 10/19/2016 04:29 PM, Dan Williams wrote:
>> On Wed, Oct 19, 2016 at 12:41 PM, Dan Williams <address@hidden> wrote:
>>> On Wed, Oct 19, 2016 at 11:42 AM, Eric Blake <address@hidden> wrote:
>>>> On 10/19/2016 11:47 AM, Dan Williams wrote:
>>>>> The 4.9 kernel added support for sub-dividing PMEM.  With this kernel
>>>>> patch [1] on top of that baseline, the PMEM-sub-division support can be
>>>>> enabled for QEMU-KVM and any other platforms that advertise both 
>>>>> un-aliased
>>>>> PMEM regions and support for the label DSM commands [2].
>>>>>
>>>>> Given this increasing need to perform a label management operation
>>>>> across a set of DIMMs this update also adds glob(3) support.  For
>>>>> example you can now write commands like:
>>>>>
>>>>>     ndctl zero-labels nmem[2-4]
>>>>
>>>> This is slightly scary, as it depends on the user not having any file
>>>> named nmem2, nmem3, or nmem4 in the current working directory.  Your
>>>> example should probably encourage proper shell quoting, as in:
>>>>
>>>> ndctl zero-labels 'nmem[2-4]'
>>>
>>> True.  Although, the glob is run against the list of present device
>>> names in the system, so local files named nmem should change the
>>> operation of the command.
>>
>> s/should/shouldn't/
>
> You didn't get my complaint.  So let me demonstrate, using echo instead
> of ndctl:
>
> $ mkdir /tmp/foo
> $ cd /tmp/foo
> $ echo nmem[2-4]
> nmem[2-4]
> $ touch nmem3 nmem4
> $ echo nmem[2-4]
> nmem3 nmem4
> $ echo 'nmem[2-4]'
> nmem[2-4]
>
> The problem is that the glob is liable to pre-expansion by the shell
> UNLESS the user quotes the glob; meaning that the current working
> directory controls whether ndctl even sees a glob in the first place.
> If you are going to support globbing in ndctl out of laziness (since it
> is indeed fewer characters to type [2-4] than it is to type {2..4}, plus
> {2..4} is not supported by all shells), then you HAVE to document that
> the user is responsible for quoting (omitting quoting usually does what
> you want).  But by the time you quote to get the glob down to ndctl,
> '[2-4]' is more typing than {2..4} expanded by the shell, at which point
> were you really being lazy by adding globbing?

Ah true, and I agree not worth it to document the need to add quoting
versus just relying on shells to do expansion.

Thanks for the feedback, I'll rip that glob support out.