[Spice-devel] [PATCH spice v2 1/1] rename the virtio port for streaming

[Frediano Ziglio]

> 
> On Fri, 2018-03-23 at 11:56 -0400, Frediano Ziglio wrote:
> > > 
> > > > On 23 Mar 2018, at 15:57, Lukáš Hrázký <lhrazky at redhat.com> wrote:
> > > > 
> > > > From: Lukáš Hrázký <lukkash at email.cz>
> > > > 
> > > > The name 'com.redhat.stream.0' is too generic and in no way denotes it
> > > > belongs to SPICE. It is preferred to have the project's domain in the
> > > > name and Red Hat doesn't own the project. Rename it to
> > > > org.spice-space.stream.0.
> > > > 
> > > > Signed-off-by: Lukáš Hrázký <lukkash at email.cz>
> > > > ---
> > > > server/reds.c                     | 2 +-
> > > > server/tests/test-stream-device.c | 2 +-
> > > > spice-common                      | 2 +-
> > > > 3 files changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/server/reds.c b/server/reds.c
> > > > index 998f2ffa..935448d8 100644
> > > > --- a/server/reds.c
> > > > +++ b/server/reds.c
> > > > @@ -3145,7 +3145,7 @@ static int
> > > > spice_server_char_device_add_interface(SpiceServer *reds,
> > > >     else if (strcmp(char_device->subtype, SUBTYPE_PORT) == 0) {
> > > >         if (strcmp(char_device->portname, "org.spice-space.webdav.0")
> > > >         == 0)
> > > >         {
> > > >             dev_state = spicevmc_device_connect(reds, char_device,
> > > >             SPICE_CHANNEL_WEBDAV);
> > > > -        } else if (strcmp(char_device->portname,
> > > > "com.redhat.stream.0") ==
> > > > 0) {
> > > > +        } else if (strcmp(char_device->portname,
> > > > "org.spice-space.stream.0") == 0) {
> > > 
> > > In addition to Frediano’s remarks, I would probably accept the two names
> > > during some transition period. Otherwise, we are going to make everybody
> > > outside of our team miserable (and inside too, while we juggle
> > > configurations around this patch).
> > > 
> > 
> > It seems complicated. The same name goes into the guest so you would have
> > to add 2 devices in the guest, unless you want the agent to try the 2
> > names.
> 
> I think it's not a bad idea. If the server accepts both names here, it
> will work with either device name configured in the VM. Then it would
> work with either an older version of the agent or you could pass the
> device through the -p argument to the agent.
> 
> Not sure who's using this outside of the team though? And not sure how
> much trouble it will cause to us. I can add the old name here if you
> guys want it.
> 

I checked and there's no "official" version of spice-server with streaming
support. Is true we'll need to update our VMs configuration but I think
would be neat in the code, it would be bad if by mistake we did a release
with both names and this propagate in external VMs/documentation or whatsoever.

> > > >             dev_state = RED_CHAR_DEVICE(stream_device_connect(reds,
> > > >             char_device));
> > > >         } else {
> > > >             dev_state = spicevmc_device_connect(reds, char_device,
> > > >             SPICE_CHANNEL_PORT);
> > > > diff --git a/server/tests/test-stream-device.c
> > > > b/server/tests/test-stream-device.c
> > > > index 3c9209a4..2fdd0a39 100644
> > > > --- a/server/tests/test-stream-device.c
> > > > +++ b/server/tests/test-stream-device.c
> > > > @@ -107,7 +107,7 @@ static SpiceCharDeviceInterface vmc_interface = {
> > > > // this specifically creates a stream device
> > > > static SpiceCharDeviceInstance vmc_instance = {
> > > >     .subtype = "port",
> > > > -    .portname = "com.redhat.stream.0",
> > > > +    .portname = "org.spice-space.stream.0",
> > > > };
> > > > 
> > > > static uint8_t *add_stream_hdr(uint8_t *p, StreamMsgType type, uint32_t
> > > > size)
> > > > diff --git a/spice-common b/spice-common
> > > > index 4c2d0e97..45e28448 160000
> > > > --- a/spice-common
> > > > +++ b/spice-common
> > > > @@ -1 +1 @@
> > > > -Subproject commit 4c2d0e977272c5540634d24f485dd64c424f6748
> > > > +Subproject commit 45e2844845242b32b2bd8956da0dfffa91c0d856
> > 
> > Frediano
>