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

Re: [libvirt] [PATCH v2 0/4] qemu: handle ',' and '=' in VM name




On 05/04/2016 10:56 AM, Cole Robinson wrote:
> This series adds qemu cli comma escaping to several places that
> are dependent on the VM name, to enable names with embedded commas.
> 
> Patch 4 makes use of qemu -name guest=X value to allow names with
> '=' in them.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=639926
> https://bugzilla.redhat.com/show_bug.cgi?id=1276485
> 
> Note: There's likely other places that are VM name dependent that need
> comma escaping too, but this hits the mandatory ones. I've listed some
> more on the BiteSizedTasks page:
> 
> http://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for_more_command_line_values
> 
> v2:
>     Rebase to master
> 
> Cole Robinson (4):
>   qemu: command: escape commas in VM name
>   qemu: command: escape commas in secret master path
>   qemu: command: escape commas in chardev socket path
>   qemu: command: Use -name guest= if available
> 
>  src/qemu/qemu_capabilities.c                       |  2 ++
>  src/qemu/qemu_capabilities.h                       |  1 +
>  src/qemu/qemu_command.c                            | 22 ++++++++++++--------
>  tests/qemucapabilitiesdata/caps_2.1.1-1.caps       |  1 +
>  tests/qemucapabilitiesdata/caps_2.4.0-1.caps       |  1 +
>  tests/qemucapabilitiesdata/caps_2.5.0-1.caps       |  1 +
>  tests/qemucapabilitiesdata/caps_2.6.0-1.caps       |  1 +
>  .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 24 ++++++++++++++++++++++
>  .../qemuxml2argvdata/qemuxml2argv-name-escape.xml  | 18 ++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 ++
>  10 files changed, 65 insertions(+), 8 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-name-escape.xml
> 

ewww again ;-)

While I cannot imagine a reason for someone (other than testing perhaps)
to want a "," or an "=" in a vm name, I suppose since it's possible to
handle by qemu, then we should 'handle' it.

Sorry I dropped the ball after my initial pass - I guess I was wondering
if there were any other (similar) feelings!

After reading the bz, I see why you've gone with double commas, although
it wasn't apparent without that read...  So maybe each commit that adds
that double comma escape could explain why the double comma works (or
even at points in the code where it occurs?).

Searching qemu_command.c for domainChannelTargetDir and it's seems
things are covered (good)... Searching for domainLibDir and I think
there's one usage not covered - qemuBuildGraphicsVNCCommandLine uses it
to build "$path_with_domainLibDir/vnc.sock".

Of course finding these "one off" type issues makes me think at some
point in the future one of those will be used for "something" and the
double comma will be missed.  So should we generate a local "escaped"
names for domainLibDir and domainChannelTargetDir that would include the
double comma and that would be passed to the corresponding users?

I think as long as you cover the VNC case and add a few more words why
the ",," can be used on the command line, then the series seems
reasonable to me, although you may want to wait a bit to push to see if
anyone else has angst over the two changes, especially now having
"guest=$name" be the default for anyone using qemu 2.1 or later.


John


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