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

[Christophe de Dinechin]

> On 23 Mar 2018, at 16:56, Frediano Ziglio <fziglio at redhat.com> 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 don’t see how you derive that conclusion. I’m suggesting that we accept either name, not that we require both.

As an aside, I’m curious why we did not create a type or subtype instead of relying on the port name? We do three strcmp (type, suptype and name) just to figure out it’s a streaming channel, and we put restrictions on user-configurable device names.


>>>            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
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel