Re: [Spice-devel] [PATCH spice-server v2 3/3] Remove core parameter from main_dispatcher_new
Acked-by: Jonathon Jongsma
On Tue, 2019-02-12 at 21:24 +, Frediano Ziglio wrote:
> This was added in bd8771adbcf3ff34d14333cf874191e8d105f612.
> There's no reason to not use reds function instead.
> MainDispatcher needs to listen in the main thread that is the
> one provided by reds_core_* functions.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/main-dispatcher.c | 29 ++---
> server/main-dispatcher.h | 2 +-
> server/reds.c| 2 +-
> 3 files changed, 8 insertions(+), 25 deletions(-)
>
> diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
> index 99d2a6216..82b25e6e4 100644
> --- a/server/main-dispatcher.c
> +++ b/server/main-dispatcher.c
> @@ -48,7 +48,6 @@
> */
> struct MainDispatcherPrivate
> {
> -SpiceCoreInterfaceInternal *core; /* weak */
> RedsState *reds; /* weak */
> SpiceWatch *watch;
> };
> @@ -58,7 +57,6 @@ G_DEFINE_TYPE_WITH_PRIVATE(MainDispatcher,
> main_dispatcher, TYPE_DISPATCHER)
> enum {
> PROP0,
> PROP_SPICE_SERVER,
> -PROP_CORE_INTERFACE
> };
>
> static void
> @@ -73,9 +71,6 @@ main_dispatcher_get_property(GObject*object,
> case PROP_SPICE_SERVER:
> g_value_set_pointer(value, self->priv->reds);
> break;
> -case PROP_CORE_INTERFACE:
> - g_value_set_pointer(value, self->priv->core);
> -break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
> }
> @@ -93,9 +88,6 @@ main_dispatcher_set_property(GObject *object,
> case PROP_SPICE_SERVER:
> self->priv->reds = g_value_get_pointer(value);
> break;
> -case PROP_CORE_INTERFACE:
> -self->priv->core = g_value_get_pointer(value);
> -break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
> }
> @@ -121,14 +113,6 @@ main_dispatcher_class_init(MainDispatcherClass
> *klass)
> "The spice
> server associated with this dispatcher",
> G_PARAM_REA
> DWRITE |
> G_PARAM_CON
> STRUCT_ONLY));
> -
> -g_object_class_install_property(object_class,
> -PROP_CORE_INTERFACE,
> -g_param_spec_pointer("core-
> interface",
> - "core-
> interface",
> - "The
> SpiceCoreInterface server associated with this dispatcher",
> - G_PARAM_REA
> DWRITE |
> - G_PARAM_CON
> STRUCT_ONLY));
> }
>
> static void
> @@ -284,11 +268,10 @@ static void dispatcher_handle_read(int fd, int
> event, void *opaque)
> * Reds routines shouldn't be exposed. Instead reds.c should
> register the callbacks,
> * and the corresponding operations should be made only via
> main_dispatcher.
> */
> -MainDispatcher* main_dispatcher_new(RedsState *reds,
> SpiceCoreInterfaceInternal *core)
> +MainDispatcher* main_dispatcher_new(RedsState *reds)
> {
> MainDispatcher *self = g_object_new(TYPE_MAIN_DISPATCHER,
> "spice-server", reds,
> -"core-interface", core,
> "max-message-type",
> MAIN_DISPATCHER_NUM_MESSAGES,
> NULL);
> return self;
> @@ -302,10 +285,10 @@ void main_dispatcher_constructed(GObject
> *object)
> dispatcher_set_opaque(DISPATCHER(self), self);
>
> self->priv->watch =
> -self->priv->core->watch_add(self->priv->core,
> -dispatcher_get_recv_fd(DISPATCHE
> R(self)),
> -SPICE_WATCH_EVENT_READ,
> dispatcher_handle_read,
> -DISPATCHER(self));
> +reds_core_watch_add(self->priv->reds,
> +dispatcher_get_recv_fd(DISPATCHER(self))
> ,
> +SPICE_WATCH_EVENT_READ,
> dispatcher_handle_read,
> +DISPATCHER(self));
> dispatcher_register_handler(DISPATCHER(self),
> MAIN_DISPATCHER_CHANNEL_EVENT,
> main_dispatcher_handle_channel_event
> ,
> sizeof(MainDispatcherChannelEventMes
> sage), false);
> @@ -324,7 +307,7 @@ static void main_dispatcher_finalize(GObject
> *object)
> {
> MainDispatcher *self = MAIN_DISPATCHER(object);
>
> -self->priv->core->watch_remove(self->priv->core, self->priv-
> >watch);
> +reds_core_watch_remove(self->priv->reds, self->priv->watch);
> self->priv->watch = NULL;
>
Re: [Spice-devel] [PATCH spice-server v2 2/3] test-stream-device: Check monitor ID messages
Looks fine to me.
Insofar as I can ACK a patch that's partially my own code:
Acked-by: Jonathon Jongsma
:)
On Tue, 2019-02-12 at 21:24 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> Signed-off-by: Jonathon Jongsma
> Reviewed-by: Jonathon Jongsma
> ---
> server/tests/test-stream-device.c | 70
> +++
> 1 file changed, 70 insertions(+)
>
> diff --git a/server/tests/test-stream-device.c b/server/tests/test-
> stream-device.c
> index f1707d2f8..e63952acb 100644
> --- a/server/tests/test-stream-device.c
> +++ b/server/tests/test-stream-device.c
> @@ -491,6 +491,75 @@ static void
> test_stream_device_data_message(TestFixture *fixture, gconstpointer
> g_assert_cmpint(send_data_bytes, ==, 1017);
> }
>
> +static void test_display_info(TestFixture *fixture, gconstpointer
> user_data)
> +{
> +// initialize a QXL interface. This must be done before
> receiving the display info message from
> +// the stream
> +test_add_display_interface(test);
> +// qxl device supports 2 monitors
> +spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0,
> 2);
> +
> +// craft a message from the mock stream device that provides
> display info to the server for the
> +// given stream
> +static const char address[] = "pci/a/b.cde";
> +StreamMsgDeviceDisplayInfo info = {
> +.stream_id = GUINT32_TO_LE(0x01020304),
> +.device_display_id = GUINT32_TO_LE(0x0a0b0c0d),
> +.device_address_len = GUINT32_TO_LE(sizeof(address)),
> +};
> +uint8_t *p = message;
> +p = add_stream_hdr(p, STREAM_TYPE_DEVICE_DISPLAY_INFO,
> sizeof(info) + sizeof(address));
> +memcpy(p, , sizeof(info));
> +p += sizeof(info);
> +strcpy((char*)p, address);
> +p += sizeof(address);
> +
> +*message_sizes_end = p - message;
> +++message_sizes_end;
> +
> +// parse the simulated display info message from the stream
> device so the server now has display
> +// info for the mock stream device
> +test_kick();
> +
> +// build the buffer to send to the agent for display information
> +SpiceMarshaller *m = spice_marshaller_new();
> +reds_marshall_device_display_info(test->server, m);
> +int to_free;
> +size_t buf_len;
> +uint8_t *buf = spice_marshaller_linearize(m, 0, _len,
> _free);
> +
> +// check output buffer. The message that we send to the vdagent
> should combine display info for
> +// the stream device that we crafted above and the qxl device.
> +static const uint8_t expected_buffer[] = {
> +/* device count */3, 0, 0, 0,
> +
> +/* channel_id */ 0, 0, 0, 0,
> +/* monitor_id */ 0, 0, 0, 0,
> +/* device_display_id */ 0, 0, 0, 0,
> +/* device_address_len */ 10, 0, 0, 0,
> +/* device_address
> */'p','c','i','/','0','/','1','.','2', 0,
> +
> +/* channel_id */ 0, 0, 0, 0,
> +/* monitor_id */ 1, 0, 0, 0,
> +/* device_display_id */ 1, 0, 0, 0,
> +/* device_address_len */ 10, 0, 0, 0,
> +/* device_address */'p','c',
> 'i','/','0','/','1','.','2', 0,
> +
> +/* channel_id */ 1, 0, 0, 0,
> +/* monitor_id */ 4, 3, 2, 1,
> +/* device_display_id */ 13, 12, 11, 10,
> +/* device_address_len */ 12, 0, 0, 0,
> +/* device_address
> */'p','c','i','/','a','/','b','.','c','d','e', 0
> +};
> +g_assert_cmpint(buf_len, ==, sizeof(expected_buffer));
> +g_assert_true(memcmp(buf, expected_buffer, buf_len) == 0);
> +
> +if (to_free) {
> +free(buf);
> +}
> +spice_marshaller_destroy(m);
> +}
> +
> static void test_add(const char *name, void (*func)(TestFixture *,
> gconstpointer), gconstpointer arg)
> {
> g_test_add(name, TestFixture, arg, test_stream_device_setup,
> func, test_stream_device_teardown);
> @@ -516,6 +585,7 @@ int main(int argc, char *argv[])
> test_stream_device_huge_data, NULL);
> test_add("/server/stream-device-data-message",
> test_stream_device_data_message, NULL);
> +test_add("/server/display-info", test_display_info, NULL);
>
> return g_test_run();
> }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server v2 1/3] reds: Factor out a function to marshal VDAgentGraphicsDeviceInfo message
Instead of scanning the monitor twice (one to compute the size
and another to build the message) use a single function to
marshal the message.
This also fixes big endian machines (which are not supported).
Marshal function is exported to make easier to test (see following
patch).
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/reds.c | 143 ++
server/reds.h | 3 ++
2 files changed, 66 insertions(+), 80 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index d0efeb782..b73253947 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -897,78 +897,33 @@ static RedPipeItem
*vdi_port_read_one_msg_from_device(RedCharDevice *self,
return NULL;
}
-void reds_send_device_display_info(RedsState *reds)
+void reds_marshall_device_display_info(RedsState *reds, SpiceMarshaller *m)
{
-if (!reds->agent_dev->priv->agent_attached) {
-return;
-}
-g_debug("Sending device display info to the agent:");
-
QXLInstance *qxl;
RedCharDevice *dev;
-size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
-
-// size for the qxl device info
-FOREACH_QXL_INSTANCE(reds, qxl) {
-message_size +=
-(sizeof(VDAgentDeviceDisplayInfo) +
strlen(red_qxl_get_device_address(qxl)) + 1) *
-red_qxl_get_monitors_count(qxl);
-}
-
-// size for the stream device info
-GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
-if (IS_STREAM_DEVICE(dev)) {
-size_t device_address_len =
-
strlen(stream_device_get_device_display_info(STREAM_DEVICE(dev))->device_address);
-
-if (device_address_len == 0) {
-// the device info wasn't set (yet), don't send it
-continue;
-}
-
-message_size += (sizeof(VDAgentDeviceDisplayInfo) +
device_address_len + 1);
-}
-}
-
-RedCharDeviceWriteBuffer *char_dev_buf =
vdagent_new_write_buffer(reds->agent_dev,
- VD_AGENT_GRAPHICS_DEVICE_INFO,
- message_size,
- true);
-
-if (!char_dev_buf) {
-reds->pending_device_display_info_message = true;
-return;
-}
-
-VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf->buf;
-VDAgentGraphicsDeviceInfo *graphics_device_info =
_buf->u.graphics_device_info;
-graphics_device_info->count = 0;
-
-VDAgentDeviceDisplayInfo *device_display_info =
graphics_device_info->display_info;
+uint32_t device_count = 0;
+void *device_count_ptr = spice_marshaller_add_uint32(m, device_count);
// add the qxl devices to the message
FOREACH_QXL_INSTANCE(reds, qxl) {
+const char *const device_address = red_qxl_get_device_address(qxl);
+const size_t device_address_len = strlen(device_address) + 1;
+if (device_address_len == 1) {
+continue;
+}
for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i) {
-device_display_info->channel_id = qxl->id;
-device_display_info->monitor_id = i;
-device_display_info->device_display_id =
red_qxl_get_device_display_ids(qxl)[i];
-
-strcpy((char*) device_display_info->device_address,
red_qxl_get_device_address(qxl));
-
-device_display_info->device_address_len =
-strlen((char*) device_display_info->device_address) + 1;
-
-g_debug(" (qxl)channel_id: %u monitor_id: %u,
device_address: %s, device_display_id: %u",
-device_display_info->channel_id,
-device_display_info->monitor_id,
-device_display_info->device_address,
-device_display_info->device_display_id);
-
-device_display_info = (VDAgentDeviceDisplayInfo*) ((char*)
device_display_info +
-sizeof(VDAgentDeviceDisplayInfo) +
device_display_info->device_address_len);
-
-graphics_device_info->count++;
+spice_marshaller_add_uint32(m, qxl->id);
+spice_marshaller_add_uint32(m, i);
+spice_marshaller_add_uint32(m,
red_qxl_get_device_display_ids(qxl)[i]);
+spice_marshaller_add_uint32(m, device_address_len);
+spice_marshaller_add(m, (void*) device_address,
device_address_len);
+++device_count;
+
+g_debug(" (qxl)channel_id: %u monitor_id: %zu,
device_address: %s, "
+"device_display_id: %u",
+qxl->id, i, device_address,
+red_qxl_get_device_display_ids(qxl)[i]);
}
}
@@ -977,9 +932,9 @@ void reds_send_device_display_info(RedsState *reds)
if (IS_STREAM_DEVICE(dev)) {
StreamDevice *stream_dev = STREAM_DEVICE(dev);
const StreamDeviceDisplayInfo *info =
stream_device_get_device_display_info(stream_dev);
-
[Spice-devel] [PATCH spice-server v2 3/3] Remove core parameter from main_dispatcher_new
This was added in bd8771adbcf3ff34d14333cf874191e8d105f612.
There's no reason to not use reds function instead.
MainDispatcher needs to listen in the main thread that is the
one provided by reds_core_* functions.
Signed-off-by: Frediano Ziglio
---
server/main-dispatcher.c | 29 ++---
server/main-dispatcher.h | 2 +-
server/reds.c| 2 +-
3 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
index 99d2a6216..82b25e6e4 100644
--- a/server/main-dispatcher.c
+++ b/server/main-dispatcher.c
@@ -48,7 +48,6 @@
*/
struct MainDispatcherPrivate
{
-SpiceCoreInterfaceInternal *core; /* weak */
RedsState *reds; /* weak */
SpiceWatch *watch;
};
@@ -58,7 +57,6 @@ G_DEFINE_TYPE_WITH_PRIVATE(MainDispatcher, main_dispatcher,
TYPE_DISPATCHER)
enum {
PROP0,
PROP_SPICE_SERVER,
-PROP_CORE_INTERFACE
};
static void
@@ -73,9 +71,6 @@ main_dispatcher_get_property(GObject*object,
case PROP_SPICE_SERVER:
g_value_set_pointer(value, self->priv->reds);
break;
-case PROP_CORE_INTERFACE:
- g_value_set_pointer(value, self->priv->core);
-break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
}
@@ -93,9 +88,6 @@ main_dispatcher_set_property(GObject *object,
case PROP_SPICE_SERVER:
self->priv->reds = g_value_get_pointer(value);
break;
-case PROP_CORE_INTERFACE:
-self->priv->core = g_value_get_pointer(value);
-break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
}
@@ -121,14 +113,6 @@ main_dispatcher_class_init(MainDispatcherClass *klass)
"The spice server
associated with this dispatcher",
G_PARAM_READWRITE |
G_PARAM_CONSTRUCT_ONLY));
-
-g_object_class_install_property(object_class,
-PROP_CORE_INTERFACE,
-g_param_spec_pointer("core-interface",
- "core-interface",
- "The
SpiceCoreInterface server associated with this dispatcher",
- G_PARAM_READWRITE |
-
G_PARAM_CONSTRUCT_ONLY));
}
static void
@@ -284,11 +268,10 @@ static void dispatcher_handle_read(int fd, int event,
void *opaque)
* Reds routines shouldn't be exposed. Instead reds.c should register the
callbacks,
* and the corresponding operations should be made only via main_dispatcher.
*/
-MainDispatcher* main_dispatcher_new(RedsState *reds,
SpiceCoreInterfaceInternal *core)
+MainDispatcher* main_dispatcher_new(RedsState *reds)
{
MainDispatcher *self = g_object_new(TYPE_MAIN_DISPATCHER,
"spice-server", reds,
-"core-interface", core,
"max-message-type",
MAIN_DISPATCHER_NUM_MESSAGES,
NULL);
return self;
@@ -302,10 +285,10 @@ void main_dispatcher_constructed(GObject *object)
dispatcher_set_opaque(DISPATCHER(self), self);
self->priv->watch =
-self->priv->core->watch_add(self->priv->core,
-dispatcher_get_recv_fd(DISPATCHER(self)),
-SPICE_WATCH_EVENT_READ,
dispatcher_handle_read,
-DISPATCHER(self));
+reds_core_watch_add(self->priv->reds,
+dispatcher_get_recv_fd(DISPATCHER(self)),
+SPICE_WATCH_EVENT_READ, dispatcher_handle_read,
+DISPATCHER(self));
dispatcher_register_handler(DISPATCHER(self),
MAIN_DISPATCHER_CHANNEL_EVENT,
main_dispatcher_handle_channel_event,
sizeof(MainDispatcherChannelEventMessage),
false);
@@ -324,7 +307,7 @@ static void main_dispatcher_finalize(GObject *object)
{
MainDispatcher *self = MAIN_DISPATCHER(object);
-self->priv->core->watch_remove(self->priv->core, self->priv->watch);
+reds_core_watch_remove(self->priv->reds, self->priv->watch);
self->priv->watch = NULL;
G_OBJECT_CLASS(main_dispatcher_parent_class)->finalize(object);
}
diff --git a/server/main-dispatcher.h b/server/main-dispatcher.h
index 088a5c216..e1244f836 100644
--- a/server/main-dispatcher.h
+++ b/server/main-dispatcher.h
@@ -59,6 +59,6 @@ void main_dispatcher_set_mm_time_latency(MainDispatcher
*self, RedClient *client
*/
void
[Spice-devel] [PATCH spice-server v2 2/3] test-stream-device: Check monitor ID messages
Signed-off-by: Frediano Ziglio
Signed-off-by: Jonathon Jongsma
Reviewed-by: Jonathon Jongsma
---
server/tests/test-stream-device.c | 70 +++
1 file changed, 70 insertions(+)
diff --git a/server/tests/test-stream-device.c
b/server/tests/test-stream-device.c
index f1707d2f8..e63952acb 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -491,6 +491,75 @@ static void test_stream_device_data_message(TestFixture
*fixture, gconstpointer
g_assert_cmpint(send_data_bytes, ==, 1017);
}
+static void test_display_info(TestFixture *fixture, gconstpointer user_data)
+{
+// initialize a QXL interface. This must be done before receiving the
display info message from
+// the stream
+test_add_display_interface(test);
+// qxl device supports 2 monitors
+spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0, 2);
+
+// craft a message from the mock stream device that provides display info
to the server for the
+// given stream
+static const char address[] = "pci/a/b.cde";
+StreamMsgDeviceDisplayInfo info = {
+.stream_id = GUINT32_TO_LE(0x01020304),
+.device_display_id = GUINT32_TO_LE(0x0a0b0c0d),
+.device_address_len = GUINT32_TO_LE(sizeof(address)),
+};
+uint8_t *p = message;
+p = add_stream_hdr(p, STREAM_TYPE_DEVICE_DISPLAY_INFO, sizeof(info) +
sizeof(address));
+memcpy(p, , sizeof(info));
+p += sizeof(info);
+strcpy((char*)p, address);
+p += sizeof(address);
+
+*message_sizes_end = p - message;
+++message_sizes_end;
+
+// parse the simulated display info message from the stream device so the
server now has display
+// info for the mock stream device
+test_kick();
+
+// build the buffer to send to the agent for display information
+SpiceMarshaller *m = spice_marshaller_new();
+reds_marshall_device_display_info(test->server, m);
+int to_free;
+size_t buf_len;
+uint8_t *buf = spice_marshaller_linearize(m, 0, _len, _free);
+
+// check output buffer. The message that we send to the vdagent should
combine display info for
+// the stream device that we crafted above and the qxl device.
+static const uint8_t expected_buffer[] = {
+/* device count */3, 0, 0, 0,
+
+/* channel_id */ 0, 0, 0, 0,
+/* monitor_id */ 0, 0, 0, 0,
+/* device_display_id */ 0, 0, 0, 0,
+/* device_address_len */ 10, 0, 0, 0,
+/* device_address */'p','c','i','/','0','/','1','.','2', 0,
+
+/* channel_id */ 0, 0, 0, 0,
+/* monitor_id */ 1, 0, 0, 0,
+/* device_display_id */ 1, 0, 0, 0,
+/* device_address_len */ 10, 0, 0, 0,
+/* device_address */'p','c', 'i','/','0','/','1','.','2', 0,
+
+/* channel_id */ 1, 0, 0, 0,
+/* monitor_id */ 4, 3, 2, 1,
+/* device_display_id */ 13, 12, 11, 10,
+/* device_address_len */ 12, 0, 0, 0,
+/* device_address */'p','c','i','/','a','/','b','.','c','d','e', 0
+};
+g_assert_cmpint(buf_len, ==, sizeof(expected_buffer));
+g_assert_true(memcmp(buf, expected_buffer, buf_len) == 0);
+
+if (to_free) {
+free(buf);
+}
+spice_marshaller_destroy(m);
+}
+
static void test_add(const char *name, void (*func)(TestFixture *,
gconstpointer), gconstpointer arg)
{
g_test_add(name, TestFixture, arg, test_stream_device_setup, func,
test_stream_device_teardown);
@@ -516,6 +585,7 @@ int main(int argc, char *argv[])
test_stream_device_huge_data, NULL);
test_add("/server/stream-device-data-message",
test_stream_device_data_message, NULL);
+test_add("/server/display-info", test_display_info, NULL);
return g_test_run();
}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server v2 0/3] Miscellaneous patches
First 2 patches are related. Changes since v1: - removed merged ones; - changed test for monitor ID adding some follow ups (and minor updated like "recieving" typo and indentation). Frediano Ziglio (3): reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message test-stream-device: Check monitor ID messages Remove core parameter from main_dispatcher_new server/main-dispatcher.c | 29 ++ server/main-dispatcher.h | 2 +- server/reds.c | 145 +- server/reds.h | 3 + server/tests/test-stream-device.c | 70 +++ 5 files changed, 144 insertions(+), 105 deletions(-) -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 6/8] agent-msg-filter: Simplify code
why not
Acked-by: Jonathon Jongsma
On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote:
> Most of the time result is set to AGENT_MSG_FILTER_OK, set at
> the beginning and change if necessary.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/agent-msg-filter.c | 14 +-
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
> index 458f563b7..abead87a5 100644
> --- a/server/agent-msg-filter.c
> +++ b/server/agent-msg-filter.c
> @@ -82,35 +82,31 @@ data_to_read:
> if (filter->discard_all) {
> filter->result = AGENT_MSG_FILTER_DISCARD;
> } else {
> +// default, easier to set once
> +filter->result = AGENT_MSG_FILTER_OK;
> switch (msg_header.type) {
> case VD_AGENT_CLIPBOARD:
> case VD_AGENT_CLIPBOARD_GRAB:
> case VD_AGENT_CLIPBOARD_REQUEST:
> case VD_AGENT_CLIPBOARD_RELEASE:
> -if (filter->copy_paste_enabled) {
> -filter->result = AGENT_MSG_FILTER_OK;
> -} else {
> +if (!filter->copy_paste_enabled) {
> filter->result = AGENT_MSG_FILTER_DISCARD;
> }
> break;
> case VD_AGENT_FILE_XFER_START:
> case VD_AGENT_FILE_XFER_STATUS:
> case VD_AGENT_FILE_XFER_DATA:
> -if (filter->file_xfer_enabled) {
> -filter->result = AGENT_MSG_FILTER_OK;
> -} else {
> +if (!filter->file_xfer_enabled) {
> filter->result = AGENT_MSG_FILTER_DISCARD;
> }
> break;
> case VD_AGENT_MONITORS_CONFIG:
> if (filter->use_client_monitors_config) {
> filter->result = AGENT_MSG_FILTER_MONITORS_CONFIG;
> -} else {
> -filter->result = AGENT_MSG_FILTER_OK;
> }
> break;
> default:
> -filter->result = AGENT_MSG_FILTER_OK;
> +break;
> }
> }
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 3/8] test-stream-device: Check monitor ID messages
Looks mostly good, but I found an issue and had a couple suggested
improvments. So I sent a couple follow-up patches that you can squash
with this patch if you think they're valid.
Reviewed-by: Jonathon Jongsma
On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> server/tests/test-stream-device.c | 52
> +++
> 1 file changed, 52 insertions(+)
>
> diff --git a/server/tests/test-stream-device.c b/server/tests/test-
> stream-device.c
> index f1707d2f8..cb3a1a4ff 100644
> --- a/server/tests/test-stream-device.c
> +++ b/server/tests/test-stream-device.c
> @@ -491,6 +491,57 @@ static void
> test_stream_device_data_message(TestFixture *fixture, gconstpointer
> g_assert_cmpint(send_data_bytes, ==, 1017);
> }
>
> +static void test_display_info(TestFixture *fixture, gconstpointer
> user_data)
> +{
> +// build a message for the streaming device
> +static const char address[] = "pci/a/b.cde";
> +StreamMsgDeviceDisplayInfo info = {
> +.stream_id = GUINT32_TO_LE(0x01020304),
> +.device_display_id = GUINT32_TO_LE(0x0a0b0c0d),
> +.device_address_len = GUINT32_TO_LE(sizeof(address)),
> +};
> +uint8_t *p = message;
> +p = add_stream_hdr(p, STREAM_TYPE_DEVICE_DISPLAY_INFO,
> sizeof(info) + sizeof(address));
> +memcpy(p, , sizeof(info));
> +p += sizeof(info);
> +strcpy((char*)p, address);
> +p += sizeof(address);
> +
> +*message_sizes_end = p - message;
> +++message_sizes_end;
> +
> +// parse the message we crafted
> +test_kick();
> +
> +// initialize a QXL interface
> +test_add_display_interface(test);
> +spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0,
> 2);
> +
> +// build the buffer to send to the agent for display information
> +SpiceMarshaller *m = spice_marshaller_new();
> +reds_marshall_device_display_info(test->server, m);
> +int to_free;
> +size_t buf_len;
> +uint8_t *buf = spice_marshaller_linearize(m, 0, _len,
> _free);
> +
> +// check output buffer
> +static const uint8_t expected_buffer[] = {
> + 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> 0,
> + 10, 0, 0, 0,112, 99,105, 47, 48, 47, 49, 46,
> 50, 0, 0, 0,
> + 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 10, 0, 0, 0,112,
> 99,
> +105, 47, 48, 47, 49, 46,
> 50, 0, 0, 0, 0, 0, 4, 3, 2, 1,
> + 13, 12, 11, 10, 12, 0, 0, 0,112, 99,105, 47, 97, 47, 98,
> 46,
> + 99,100,101, 0
> +};
> +g_assert_cmpint(buf_len, ==, sizeof(expected_buffer));
> +g_assert_true(memcmp(buf, expected_buffer, buf_len) == 0);
> +
> +if (to_free) {
> +free(buf);
> +}
> +spice_marshaller_destroy(m);
> +}
> +
> static void test_add(const char *name, void (*func)(TestFixture *,
> gconstpointer), gconstpointer arg)
> {
> g_test_add(name, TestFixture, arg, test_stream_device_setup,
> func, test_stream_device_teardown);
> @@ -516,6 +567,7 @@ int main(int argc, char *argv[])
> test_stream_device_huge_data, NULL);
> test_add("/server/stream-device-data-message",
> test_stream_device_data_message, NULL);
> +test_add("/server/display-info", test_display_info, NULL);
>
> return g_test_run();
> }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server 1/2] Fixup display info test
Rather than showing the expected data in raw format (ascii codes, etc),
which is hard to verify, show the characters themselves, and group them
by structure.
Also add a few more comments.
---
server/tests/test-stream-device.c | 35 +++
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/server/tests/test-stream-device.c
b/server/tests/test-stream-device.c
index cb3a1a4ff..dcdd91896 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -493,7 +493,8 @@ static void test_stream_device_data_message(TestFixture
*fixture, gconstpointer
static void test_display_info(TestFixture *fixture, gconstpointer user_data)
{
-// build a message for the streaming device
+// craft a message from the mock stream device that provides display info
to the server for the
+// given stream
static const char address[] = "pci/a/b.cde";
StreamMsgDeviceDisplayInfo info = {
.stream_id = GUINT32_TO_LE(0x01020304),
@@ -510,11 +511,13 @@ static void test_display_info(TestFixture *fixture,
gconstpointer user_data)
*message_sizes_end = p - message;
++message_sizes_end;
-// parse the message we crafted
+// parse the simulated display info message from the stream device so the
server now has display
+// info for the mock stream device
test_kick();
// initialize a QXL interface
test_add_display_interface(test);
+/* qxl device supports 2 monitors */
spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0, 2);
// build the buffer to send to the agent for display information
@@ -524,14 +527,28 @@ static void test_display_info(TestFixture *fixture,
gconstpointer user_data)
size_t buf_len;
uint8_t *buf = spice_marshaller_linearize(m, 0, _len, _free);
-// check output buffer
+// check output buffer. The message that we send to the vdagent should
combine display info for
+// the stream device that we crafted above and the qxl device.
static const uint8_t expected_buffer[] = {
- 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
- 10, 0, 0, 0,112, 99,105, 47, 48, 47, 49, 46, 50, 0, 0, 0,
- 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 10, 0, 0, 0,112, 99,
-105, 47, 48, 47, 49, 46, 50, 0, 0, 0, 0, 0, 4, 3, 2, 1,
- 13, 12, 11, 10, 12, 0, 0, 0,112, 99,105, 47, 97, 47, 98, 46,
- 99,100,101, 0
+ /* device count */3, 0, 0, 0,
+
+ /* channel_id */ 0, 0, 0, 0,
+ /* monitor_id */ 0, 0, 0, 0,
+ /* device_display_id */ 0, 0, 0, 0,
+ /* device_address_len */ 10, 0, 0, 0,
+ /* device_address */'p','c','i','/','0','/','1','.','2', 0,
+
+ /* channel_id */ 0, 0, 0, 0,
+ /* monitor_id */ 1, 0, 0, 0,
+ /* device_display_id */ 1, 0, 0, 0,
+ /* device_address_len */ 10, 0, 0, 0,
+ /* device_address */'p','c', 'i','/','0','/','1','.','2', 0,
+
+ /* channel_id */ 0, 0, 0, 0,
+ /* monitor_id */ 4, 3, 2, 1,
+ /* device_display_id */ 13, 12, 11, 10,
+ /* device_address_len */ 12, 0, 0, 0,
+ /* device_address */'p','c','i','/','a','/','b','.','c','d','e',
0
};
g_assert_cmpint(buf_len, ==, sizeof(expected_buffer));
g_assert_true(memcmp(buf, expected_buffer, buf_len) == 0);
--
2.17.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server 2/2] QXL devices must be registered before Stream Devices
Stream devices assume that all QXL devices are registered with the
server before we receive any communications from the stream device. This
is due to the fact that QXL display channel IDs are assigned directly
from the QXL device ID, whereas Stream display channels are assigned
channel IDs based on the next free ID. If the stream channel is
created first, it will find that 0 is the first available display ID and
use that for its channel ID. Then when QXL device #0 is registered, it
will also create a display channel with channel ID 0, and they will
conflict.
---
server/tests/test-stream-device.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/server/tests/test-stream-device.c
b/server/tests/test-stream-device.c
index dcdd91896..86c33a145 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -493,6 +493,12 @@ static void test_stream_device_data_message(TestFixture
*fixture, gconstpointer
static void test_display_info(TestFixture *fixture, gconstpointer user_data)
{
+// initialize a QXL interface. This must be done before recieving the
display info message from
+// the stream
+test_add_display_interface(test);
+/* qxl device supports 2 monitors */
+spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0, 2);
+
// craft a message from the mock stream device that provides display info
to the server for the
// given stream
static const char address[] = "pci/a/b.cde";
@@ -515,11 +521,6 @@ static void test_display_info(TestFixture *fixture,
gconstpointer user_data)
// info for the mock stream device
test_kick();
-// initialize a QXL interface
-test_add_display_interface(test);
-/* qxl device supports 2 monitors */
-spice_qxl_set_device_info(>qxl_instance, "pci/0/1.2", 0, 2);
-
// build the buffer to send to the agent for display information
SpiceMarshaller *m = spice_marshaller_new();
reds_marshall_device_display_info(test->server, m);
@@ -544,7 +545,7 @@ static void test_display_info(TestFixture *fixture,
gconstpointer user_data)
/* device_address_len */ 10, 0, 0, 0,
/* device_address */'p','c', 'i','/','0','/','1','.','2', 0,
- /* channel_id */ 0, 0, 0, 0,
+ /* channel_id */ 1, 0, 0, 0,
/* monitor_id */ 4, 3, 2, 1,
/* device_display_id */ 13, 12, 11, 10,
/* device_address_len */ 12, 0, 0, 0,
--
2.17.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable
From: Victor Toso
To keep all dependencies together. Some (small) effort was made to
distinguish what is necessary for Fedora and what is necessary for
Windows builds in order to install only required packages when job is
executing.
Note that we are adding gettext, gettext-devel and glib2-devel
explicit now on DEPS_COMMON. The reason is that, both Fedora and mingw
builds require some tooling but for Fedora it gets installed
indirectly while for Mingw, it doesn't and configure/build would fail.
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 28 +---
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aab5f77..2627241 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,18 +1,24 @@
image: fedora:latest
variables:
- DEPS: git libtool make python3 python3-six redhat-rpm-config
-python3-pyparsing meson ninja-build zlib-devel openssl-devel
-intltool gtk3-devel gtk-doc gobject-introspection-devel
-cyrus-sasl-devel pulseaudio-libs-devel libjpeg-turbo-devel
-libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
-polkit-devel vala lz4-devel opus-devel
-pixman-devel libcacard-devel celt051-devel libphodav-devel
-usbutils usbredir-devel libusbx-devel libsoup-devel
-json-glib-devel
+ DEPS_COMMON: git libtool make python3 python3-six redhat-rpm-config
+ python3-pyparsing meson ninja-build gtk-doc glib2-devel
+ gettext gettext-devel
+
+ DEPS_FEDORA: zlib-devel openssl-devel intltool gtk3-devel
+ gobject-introspection-devel cyrus-sasl-devel
+ pulseaudio-libs-devel libjpeg-turbo-devel
+ libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
+ polkit-devel vala lz4-devel opus-devel pixman-devel
+ libcacard-devel celt051-devel libphodav-devel usbutils
+ usbredir-devel libusbx-devel libsoup-devel json-glib-devel
+
+ DEPS_MINGW: mingw64-gcc mingw64-pkg-config mingw64-pixman mingw64-openssl
+ mingw64-gtk3 mingw64-json-glib mingw64-opus
+ mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
before_script:
- - dnf install -y $DEPS
+ - dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
@@ -47,7 +53,7 @@ fedora-meson:
windows-autotools:
script:
-- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
+- dnf install -y $DEPS_COMMON $DEPS_MINGW
- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
- NOCONFIGURE=yes ./autogen.sh
- PYTHON=python3 mingw64-configure --enable-static
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message
> On Tue, 2019-02-12 at 04:05 -0500, Frediano Ziglio wrote:
> >
> > > Untested, but looks fine.
> > >
> > > Acked-by: Jonathon Jongsma
> > >
> >
> > It's partially tested by following patch. Partially as the new
> > function is
> > tested but the old function to send the message is not but is changed
> > in
> > this test.
> > But I can see data sent to the guest (so I tested that part
> > manually).
> >
> > Should the verb be "marshall" or "marshal" ? I think the second, so
> > I should rename the function.
>
> Yeah, I believe marshal is technically correct
> Jonathon
>
I did some grep+wc on our code... looks like we are changing the dictionary!
I think 90% or more of the occurrences are marshalL.
But we have to start fixing it somewhere, why not now?
Looks like both marshaler and marshaller are correct although the later
(marshaller) seems more accepted.
>
> >
> > Frediano
> >
> > >
> > >
> > > On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote:
> > > > Instead of scanning the monitor twice (one to compute the size
> > > > and another to build the message) use a single function to
> > > > marshall the message.
> > > > This also fixes big endian machines (which are not supported).
> > > > Marshall function is exported to make easier to test (see
> > > > following
> > > > patch).
> > > >
> > > > Signed-off-by: Frediano Ziglio
> > > > ---
> > > > Also the reds_marshall_device_display_info function will be
> > > > potentially used to separate VDIPort code.
> > > > ---
> > > > server/reds.c | 143 ++
> > > >
> > > >
> > > > server/reds.h | 3 ++
> > > > 2 files changed, 66 insertions(+), 80 deletions(-)
> > > >
> > > > diff --git a/server/reds.c b/server/reds.c
> > > > index 86f020a87..c16655adf 100644
> > > > --- a/server/reds.c
> > > > +++ b/server/reds.c
> > > > @@ -897,78 +897,33 @@ static RedPipeItem
> > > > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > > > return NULL;
> > > > }
> > > >
> > > > -void reds_send_device_display_info(RedsState *reds)
> > > > +void reds_marshall_device_display_info(RedsState *reds,
> > > > SpiceMarshaller *m)
> > > > {
> > > > -if (!reds->agent_dev->priv->agent_attached) {
> > > > -return;
> > > > -}
> > > > -g_debug("Sending device display info to the agent:");
> > > > -
> > > > QXLInstance *qxl;
> > > > RedCharDevice *dev;
> > > >
> > > > -size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > > > -
> > > > -// size for the qxl device info
> > > > -FOREACH_QXL_INSTANCE(reds, qxl) {
> > > > -message_size +=
> > > > -(sizeof(VDAgentDeviceDisplayInfo) +
> > > > strlen(red_qxl_get_device_address(qxl)) + 1) *
> > > > -red_qxl_get_monitors_count(qxl);
> > > > -}
> > > > -
> > > > -// size for the stream device info
> > > > -GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> > > > -if (IS_STREAM_DEVICE(dev)) {
> > > > -size_t device_address_len =
> > > > -strlen(stream_device_get_device_display_info(STR
> > > > EAM_
> > > > DEVICE(dev))->device_address);
> > > > -
> > > > -if (device_address_len == 0) {
> > > > -// the device info wasn't set (yet), don't send
> > > > it
> > > > -continue;
> > > > -}
> > > > -
> > > > -message_size += (sizeof(VDAgentDeviceDisplayInfo) +
> > > > device_address_len + 1);
> > > > -}
> > > > -}
> > > > -
> > > > -RedCharDeviceWriteBuffer *char_dev_buf =
> > > > vdagent_new_write_buffer(reds->agent_dev,
> > > > - VD_AGENT_GRAPHICS_DEVIC
> > > > E_IN
> > > > FO,
> > > > - message_size,
> > > > - true);
> > > > -
> > > > -if (!char_dev_buf) {
> > > > -reds->pending_device_display_info_message = true;
> > > > -return;
> > > > -}
> > > > -
> > > > -VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf-
> > > > > buf;
> > > >
> > > > -VDAgentGraphicsDeviceInfo *graphics_device_info =
> > > > _buf-
> > > > > u.graphics_device_info;
> > > >
> > > > -graphics_device_info->count = 0;
> > > > -
> > > > -VDAgentDeviceDisplayInfo *device_display_info =
> > > > graphics_device_info->display_info;
> > > > +uint32_t device_count = 0;
> > > > +void *device_count_ptr = spice_marshaller_add_uint32(m,
> > > > device_count);
> > > >
> > > > // add the qxl devices to the message
> > > > FOREACH_QXL_INSTANCE(reds, qxl) {
> > > > +const char *const device_address =
> > > > red_qxl_get_device_address(qxl);
> > > > +const size_t device_address_len = strlen(device_address)
> > > > +
> > > > 1;
> > > > +if (device_address_len == 1) {
> > > > +continue;
> > > > +}
> > > > for (size_t i = 0; i < red_qxl_get_monitors_count(qxl);
[Spice-devel] [spice-gtk v2 3/5] gitlab-ci: create before_script per job
From: Victor Toso
* On a windows job-build, we don't need to install Fedora dependencies.
This change makes only one dnf install be ran per job.
* On a meson build, we should build spice-protocol with meson too.
Moving this before_script rule to each job makes all of this clear.
So, this patch does change for fedora-meson, the build of
spice-protocol with meson and we don't (explicitly) install any of
$DEPS_FEDORA
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2627241..aa67ed5 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -17,12 +17,12 @@ variables:
mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
-before_script:
- - dnf install -y $DEPS_COMMON $DEPS_FEDORA
- - git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- - (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
-
fedora-autotools:
+ before_script:
+- dnf install -y $DEPS_COMMON $DEPS_FEDORA
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
+
script:
# Run with default options
- ./autogen.sh --enable-static
@@ -42,6 +42,12 @@ fedora-autotools:
- make check
fedora-meson:
+ before_script:
+- dnf install -y $DEPS_COMMON $DEPS_FEDORA
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- meson spice-protocol _build_spice-protocol --prefix=/usr
+- ninja -C _build_spice-protocol install
+
script:
- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
- ninja -C _build_default
@@ -52,9 +58,12 @@ fedora-meson:
- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
windows-autotools:
- script:
+ before_script:
- dnf install -y $DEPS_COMMON $DEPS_MINGW
-- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- (cd spice-protocol && autoreconf -if && mingw64-configure --prefix=/usr
&& make install)
+
+ script:
- NOCONFIGURE=yes ./autogen.sh
- PYTHON=python3 mingw64-configure --enable-static
- make -j4
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 4/5] gitlab-ci: add artifacts for logs and tests
From: Victor Toso
Much better than playing around with shell. Logs should live for week
since the build and CI will try to always upload them.
This patch also adds the logs for tests from builds with autotools
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aa67ed5..c68f9d7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -18,6 +18,12 @@ variables:
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
fedora-autotools:
+ artifacts:
+paths:
+ - tests/*.log
+when: always
+expire_in: 1 week
+
before_script:
- dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
@@ -42,6 +48,12 @@ fedora-autotools:
- make check
fedora-meson:
+ artifacts:
+paths:
+ - _build_*/meson-logs/*.txt
+when: always
+expire_in: 1 week
+
before_script:
- dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
@@ -49,13 +61,13 @@ fedora-meson:
- ninja -C _build_spice-protocol install
script:
-- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
+- meson _build_default
- ninja -C _build_default
-- ninja -C _build_default test || (cat
_build_default/meson-logs/testlog.txt && exit 1)
+- ninja -C _build_default test
-- meson _build_feat_disabled -Dauto_features=disabled || (cat
_build_feat_disabled/meson-logs/meson-log.txt && exit 1)
+- meson _build_feat_disabled -Dauto_features=disabled
- ninja -C _build_feat_disabled
-- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
+- ninja -C _build_feat_disabled test
windows-autotools:
before_script:
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1] widget: mouse: Fix getting coordinates
From: Victor Toso
Documentation for gdk_display_get_primary_monitor() says that it
returns "the primary monitor, or NULL if no primary monitor is
configured by the user".
If monitor endup being NULL, we endup using unitialized GdkRectangle
geom later on as gdk_monitor_get_geometry() will fail with:
| gdk_monitor_get_geometry: assertion 'GDK_IS_MONITOR (monitor)' failed
This patch tries gdk_display_get_monitor_at_point() and will fail if
no GdkMonitor is set.
Signed-off-by: Victor Toso
---
src/spice-widget.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 8adcc38..e69b808 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1129,6 +1129,11 @@ static void mouse_wrap(SpiceDisplay *display,
GdkEventMotion *motion)
GdkWindow *gdk_window = gtk_widget_get_window(GTK_WIDGET(display));
GdkDisplay *gdk_display = gdk_window_get_display(gdk_window);
GdkMonitor *monitor = gdk_display_get_primary_monitor(gdk_display);
+if (monitor == NULL) {
+/* No primary monitor set, using last mouse coordinates */
+monitor = gdk_display_get_monitor_at_point(gdk_display,
d->mouse_last_x, d->mouse_last_y);
+}
+g_return_if_fail(monitor != NULL);
gdk_monitor_get_geometry(monitor, );
xr = geom.width / 2;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
> Hi, > > On Tue, Feb 12, 2019 at 11:19:16AM -0500, Frediano Ziglio wrote: > > > > -makecheck-meson: > > > > +fedora-meson: > > > >script: > > > > - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) > > > > - - ninja -C build > > > > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && > > > > exit > > > > 1) > > > > +- meson _build_default || (cat > > > > _build_default/meson-logs/meson-log.txt > > > > && exit 1) > > > > > > Why the change to _build_default instead of build? > > > I suppose to group them but why the initial underscore? > > > Why not deleting the directory every time to reduce disk space? > > > > > > > I got also the multiple directories, in the last patch you same > > the logs as artifacts! > > > > But still I don't get the initial underscore. > > Maybe some unwritten rule. It is not usual to have initial '_' in > projects, so it is good for temporary folders. I've seen that in > other projects that I contribute and I thought in try it here, I > don't mind removing it. > Not strong about. Autoconf used .build for the "make distcheck" (and maybe "make dist"), somebody complained about portability (initial "." is not portable, on Unix is considered hidden) and was no reason to hide the build (as more difficult to spot garbage directories or finding the logs). "build" was more potential to collide with other directories so they decided for "_build". I don't know other usages. Surely I don't want a directory to start with "-build", would be terrible! > > Also would be better to use dash instead to have > > build-spice-protocol and not build_spice-protocol. > > I like using '-' too, but here the distinction was > build_+$project_name. I'll change it as I really don't care > either way. > No, don't mind, not too strong about it. > Thanks for reviews. > > Ah, yes, you asked somewhere, in the cover letter there is the > link for what this looks like, etc. Adding it here again: > > https://gitlab.freedesktop.org/victortoso/spice-gtk/pipelines/18709 > Thanks. > > > > > > +- ninja -C _build_default > > > > +- ninja -C _build_default test || (cat > > > > _build_default/meson-logs/testlog.txt && exit 1) > > > > > > > > -makecheck_simple: > > > > - script: > > > > - - ./autogen.sh --enable-static > > > > ---enable-lz4=no > > > > ---enable-webdav=no > > > > ---with-sasl=no > > > > ---with-coroutine=auto > > > > ---enable-pulse=no > > > > ---enable-smartcard=no > > > > ---enable-usbredir=no > > > > - - make -j4 > > > > - - make check > > > > - > > > > -makecheck_simple-meson: > > > > - script: > > > > - - meson build -Dauto_features=disabled || (cat > > > > build/meson-logs/meson-log.txt && exit 1) > > > > - - ninja -C build > > > > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && > > > > exit > > > > 1) > > > > +- meson _build_feat_disabled -Dauto_features=disabled || (cat > > > > _build_feat_disabled/meson-logs/meson-log.txt && exit 1) > > > > +- ninja -C _build_feat_disabled > > > > +- ninja -C _build_feat_disabled test || (cat > > > > _build_feat_disabled/meson-logs/testlog.txt && exit 1) > > > > > > > > -make-win: > > > > +windows-autotools: > > > >script: > > > > - - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman > > > > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus > > > > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good > > > > - - (cd spice-protocol && make clean && mingw64-configure > > > > --prefix=/usr && > > > > make install) > > > > - - NOCONFIGURE=yes ./autogen.sh > > > > - - PYTHON=python3 mingw64-configure --enable-static > > > > - - make -j4 > > > > +- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman > > > > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus > > > > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good > > > > +- (cd spice-protocol && make clean && mingw64-configure > > > > --prefix=/usr > > > > && > > > > make install) > > > > +- NOCONFIGURE=yes ./autogen.sh > > > > +- PYTHON=python3 mingw64-configure --enable-static > > > > +- make -j4 > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
From: Victor Toso
Group by target build instead of command. The focus of each job is to
check any regression for given platform, using 'fedora'/'windows' and
'autotools'/'meson' seems more intuitive.
By doing that we are grouping similar jobs together, this is
intentional as we are reducing the amount of jobs that need to be run
(together with the whole bootstrapping) without losing logs or tests.
Some indentation takes place too, keeping it to 2 spaces as some other
places in the code
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 62 --
1 file changed, 30 insertions(+), 32 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 74280e9..aab5f77 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -16,41 +16,39 @@ before_script:
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
-makecheck:
+fedora-autotools:
script:
- - ./autogen.sh --enable-static
- - make -j4
- - make check
+# Run with default options
+- ./autogen.sh --enable-static
+- make -j4
+- make check
+# Run without features
+- git clean -xfd
+- ./autogen.sh --enable-static
+ --enable-lz4=no
+ --enable-webdav=no
+ --with-sasl=no
+ --with-coroutine=auto
+ --enable-pulse=no
+ --enable-smartcard=no
+ --enable-usbredir=no
+- make -j4
+- make check
-makecheck-meson:
+fedora-meson:
script:
- - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
- - ninja -C build
- - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
+- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
+- ninja -C _build_default
+- ninja -C _build_default test || (cat
_build_default/meson-logs/testlog.txt && exit 1)
-makecheck_simple:
- script:
- - ./autogen.sh --enable-static
---enable-lz4=no
---enable-webdav=no
---with-sasl=no
---with-coroutine=auto
---enable-pulse=no
---enable-smartcard=no
---enable-usbredir=no
- - make -j4
- - make check
-
-makecheck_simple-meson:
- script:
- - meson build -Dauto_features=disabled || (cat
build/meson-logs/meson-log.txt && exit 1)
- - ninja -C build
- - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
+- meson _build_feat_disabled -Dauto_features=disabled || (cat
_build_feat_disabled/meson-logs/meson-log.txt && exit 1)
+- ninja -C _build_feat_disabled
+- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
-make-win:
+windows-autotools:
script:
- - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
- - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
- - NOCONFIGURE=yes ./autogen.sh
- - PYTHON=python3 mingw64-configure --enable-static
- - make -j4
+- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
+- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
+- NOCONFIGURE=yes ./autogen.sh
+- PYTHON=python3 mingw64-configure --enable-static
+- make -j4
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message
On Tue, 2019-02-12 at 12:24 -0500, Frediano Ziglio wrote:
> > On Tue, 2019-02-12 at 04:05 -0500, Frediano Ziglio wrote:
> > >
> > > > Untested, but looks fine.
> > > >
> > > > Acked-by: Jonathon Jongsma
> > > >
> > >
> > > It's partially tested by following patch. Partially as the new
> > > function is
> > > tested but the old function to send the message is not but is
> > > changed
> > > in
> > > this test.
> > > But I can see data sent to the guest (so I tested that part
> > > manually).
> > >
> > > Should the verb be "marshall" or "marshal" ? I think the second,
> > > so
> > > I should rename the function.
> >
> > Yeah, I believe marshal is technically correct
> > Jonathon
> >
>
> I did some grep+wc on our code... looks like we are changing the
> dictionary!
> I think 90% or more of the occurrences are marshalL.
> But we have to start fixing it somewhere, why not now?
> Looks like both marshaler and marshaller are correct although the
> later
> (marshaller) seems more accepted.
Yeah, it's a bit of an odd case because both "marshal" and "marshall"
are valid words (though the latter is more often a family name), so
they get mixed up a lot. To be honest, I don't mind too much whether
you decide to keep the wrong spelling for consistency or start changing
them to the 'correct' spelling.
But yes, "marshaller" and "marshalling" usually have a double L due to
the normal pronunciation rules of English. "Marshaler" looks like it
should be pronounced with a long A sound. English is dumb ;)
Jonathon
>
> >
> > >
> > > Frediano
> > >
> > > >
> > > >
> > > > On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote:
> > > > > Instead of scanning the monitor twice (one to compute the
> > > > > size
> > > > > and another to build the message) use a single function to
> > > > > marshall the message.
> > > > > This also fixes big endian machines (which are not
> > > > > supported).
> > > > > Marshall function is exported to make easier to test (see
> > > > > following
> > > > > patch).
> > > > >
> > > > > Signed-off-by: Frediano Ziglio
> > > > > ---
> > > > > Also the reds_marshall_device_display_info function will be
> > > > > potentially used to separate VDIPort code.
> > > > > ---
> > > > > server/reds.c | 143 ++
> > > > >
> > > > >
> > > > >
> > > > > server/reds.h | 3 ++
> > > > > 2 files changed, 66 insertions(+), 80 deletions(-)
> > > > >
> > > > > diff --git a/server/reds.c b/server/reds.c
> > > > > index 86f020a87..c16655adf 100644
> > > > > --- a/server/reds.c
> > > > > +++ b/server/reds.c
> > > > > @@ -897,78 +897,33 @@ static RedPipeItem
> > > > > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > > > > return NULL;
> > > > > }
> > > > >
> > > > > -void reds_send_device_display_info(RedsState *reds)
> > > > > +void reds_marshall_device_display_info(RedsState *reds,
> > > > > SpiceMarshaller *m)
> > > > > {
> > > > > -if (!reds->agent_dev->priv->agent_attached) {
> > > > > -return;
> > > > > -}
> > > > > -g_debug("Sending device display info to the agent:");
> > > > > -
> > > > > QXLInstance *qxl;
> > > > > RedCharDevice *dev;
> > > > >
> > > > > -size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > > > > -
> > > > > -// size for the qxl device info
> > > > > -FOREACH_QXL_INSTANCE(reds, qxl) {
> > > > > -message_size +=
> > > > > -(sizeof(VDAgentDeviceDisplayInfo) +
> > > > > strlen(red_qxl_get_device_address(qxl)) + 1) *
> > > > > -red_qxl_get_monitors_count(qxl);
> > > > > -}
> > > > > -
> > > > > -// size for the stream device info
> > > > > -GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> > > > > -if (IS_STREAM_DEVICE(dev)) {
> > > > > -size_t device_address_len =
> > > > > -strlen(stream_device_get_device_display_info
> > > > > (STR
> > > > > EAM_
> > > > > DEVICE(dev))->device_address);
> > > > > -
> > > > > -if (device_address_len == 0) {
> > > > > -// the device info wasn't set (yet), don't
> > > > > send
> > > > > it
> > > > > -continue;
> > > > > -}
> > > > > -
> > > > > -message_size +=
> > > > > (sizeof(VDAgentDeviceDisplayInfo) +
> > > > > device_address_len + 1);
> > > > > -}
> > > > > -}
> > > > > -
> > > > > -RedCharDeviceWriteBuffer *char_dev_buf =
> > > > > vdagent_new_write_buffer(reds->agent_dev,
> > > > > - VD_AGENT_GRAPHICS_D
> > > > > EVIC
> > > > > E_IN
> > > > > FO,
> > > > > - message_size,
> > > > > - true);
> > > > > -
> > > > > -if (!char_dev_buf) {
> > > > > -reds->pending_device_display_info_message = true;
> > > > > -return;
> > > > > -}
> > > > > -
> > > > > -VDInternalBuf *internal_buf = (VDInternalBuf
> > > > >
[Spice-devel] [spice-gtk v1] widget: mouse: Fix getting coordinates
From: Victor Toso
Documentation for gdk_display_get_primary_monitor() says that it
returns "the primary monitor, or NULL if no primary monitor is
configured by the user".
If monitor endup being NULL, we endup using unitialized GdkRectangle
geom later on as gdk_monitor_get_geometry() will fail with:
| gdk_monitor_get_geometry: assertion 'GDK_IS_MONITOR (monitor)' failed
This patch tries gdk_display_get_monitor_at_point() and will fail if
no GdkMonitor is set.
Signed-off-by: Victor Toso
---
src/spice-widget.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 8adcc38..e69b808 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1129,6 +1129,11 @@ static void mouse_wrap(SpiceDisplay *display,
GdkEventMotion *motion)
GdkWindow *gdk_window = gtk_widget_get_window(GTK_WIDGET(display));
GdkDisplay *gdk_display = gdk_window_get_display(gdk_window);
GdkMonitor *monitor = gdk_display_get_primary_monitor(gdk_display);
+if (monitor == NULL) {
+/* No primary monitor set, using last mouse coordinates */
+monitor = gdk_display_get_monitor_at_point(gdk_display,
d->mouse_last_x, d->mouse_last_y);
+}
+g_return_if_fail(monitor != NULL);
gdk_monitor_get_geometry(monitor, );
xr = geom.width / 2;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
Hi, On Tue, Feb 12, 2019 at 11:19:16AM -0500, Frediano Ziglio wrote: > > > -makecheck-meson: > > > +fedora-meson: > > >script: > > > - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) > > > - - ninja -C build > > > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit > > > 1) > > > +- meson _build_default || (cat > > > _build_default/meson-logs/meson-log.txt > > > && exit 1) > > > > Why the change to _build_default instead of build? > > I suppose to group them but why the initial underscore? > > Why not deleting the directory every time to reduce disk space? > > > > I got also the multiple directories, in the last patch you same > the logs as artifacts! > > But still I don't get the initial underscore. Maybe some unwritten rule. It is not usual to have initial '_' in projects, so it is good for temporary folders. I've seen that in other projects that I contribute and I thought in try it here, I don't mind removing it. > Also would be better to use dash instead to have > build-spice-protocol and not build_spice-protocol. I like using '-' too, but here the distinction was build_+$project_name. I'll change it as I really don't care either way. Thanks for reviews. Ah, yes, you asked somewhere, in the cover letter there is the link for what this looks like, etc. Adding it here again: https://gitlab.freedesktop.org/victortoso/spice-gtk/pipelines/18709 > > > > +- ninja -C _build_default > > > +- ninja -C _build_default test || (cat > > > _build_default/meson-logs/testlog.txt && exit 1) > > > > > > -makecheck_simple: > > > - script: > > > - - ./autogen.sh --enable-static > > > ---enable-lz4=no > > > ---enable-webdav=no > > > ---with-sasl=no > > > ---with-coroutine=auto > > > ---enable-pulse=no > > > ---enable-smartcard=no > > > ---enable-usbredir=no > > > - - make -j4 > > > - - make check > > > - > > > -makecheck_simple-meson: > > > - script: > > > - - meson build -Dauto_features=disabled || (cat > > > build/meson-logs/meson-log.txt && exit 1) > > > - - ninja -C build > > > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit > > > 1) > > > +- meson _build_feat_disabled -Dauto_features=disabled || (cat > > > _build_feat_disabled/meson-logs/meson-log.txt && exit 1) > > > +- ninja -C _build_feat_disabled > > > +- ninja -C _build_feat_disabled test || (cat > > > _build_feat_disabled/meson-logs/testlog.txt && exit 1) > > > > > > -make-win: > > > +windows-autotools: > > >script: > > > - - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman > > > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus > > > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good > > > - - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr > > > && > > > make install) > > > - - NOCONFIGURE=yes ./autogen.sh > > > - - PYTHON=python3 mingw64-configure --enable-static > > > - - make -j4 > > > +- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman > > > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus > > > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good > > > +- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr > > > && > > > make install) > > > +- NOCONFIGURE=yes ./autogen.sh > > > +- PYTHON=python3 mingw64-configure --enable-static > > > +- make -j4 signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
> >
> > From: Victor Toso
> >
> > Group by target build instead of command. The focus of each job is to
> > check any regression for given platform, using 'fedora'/'windows' and
> > 'autotools'/'meson' seems more intuitive.
> >
> > By doing that we are grouping similar jobs together, this is
> > intentional as we are reducing the amount of jobs that need to be run
> > (together with the whole bootstrapping) without losing logs or tests.
> >
> > Some indentation takes place too, keeping it to 2 spaces as some other
> > places in the code
> >
>
> Looking at the code I suppose you mean 4 not 2 in the last sentence.
>
Ok, I got his, there are 2 levels so the 4 spaces below..
> > Signed-off-by: Victor Toso
> > ---
> > .gitlab-ci.yml | 62 --
> > 1 file changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 74280e9..aab5f77 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -16,41 +16,39 @@ before_script:
> >- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
> >- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
> >
> > -makecheck:
> > +fedora-autotools:
> >script:
> > - - ./autogen.sh --enable-static
> > - - make -j4
> > - - make check
> > +# Run with default options
> > +- ./autogen.sh --enable-static
> > +- make -j4
> > +- make check
> > +# Run without features
> > +- git clean -xfd
> > +- ./autogen.sh --enable-static
> > + --enable-lz4=no
> > + --enable-webdav=no
> > + --with-sasl=no
> > + --with-coroutine=auto
> > + --enable-pulse=no
> > + --enable-smartcard=no
> > + --enable-usbredir=no
> > +- make -j4
> > +- make check
> >
> > -makecheck-meson:
> > +fedora-meson:
> >script:
> > - - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
> > - - ninja -C build
> > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit
> > 1)
> > +- meson _build_default || (cat _build_default/meson-logs/meson-log.txt
> > && exit 1)
>
> Why the change to _build_default instead of build?
> I suppose to group them but why the initial underscore?
> Why not deleting the directory every time to reduce disk space?
>
I got also the multiple directories, in the last patch you same the logs
as artifacts!
But still I don't get the initial underscore.
Also would be better to use dash instead to have build-spice-protocol
and not build_spice-protocol.
> > +- ninja -C _build_default
> > +- ninja -C _build_default test || (cat
> > _build_default/meson-logs/testlog.txt && exit 1)
> >
> > -makecheck_simple:
> > - script:
> > - - ./autogen.sh --enable-static
> > ---enable-lz4=no
> > ---enable-webdav=no
> > ---with-sasl=no
> > ---with-coroutine=auto
> > ---enable-pulse=no
> > ---enable-smartcard=no
> > ---enable-usbredir=no
> > - - make -j4
> > - - make check
> > -
> > -makecheck_simple-meson:
> > - script:
> > - - meson build -Dauto_features=disabled || (cat
> > build/meson-logs/meson-log.txt && exit 1)
> > - - ninja -C build
> > - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit
> > 1)
> > +- meson _build_feat_disabled -Dauto_features=disabled || (cat
> > _build_feat_disabled/meson-logs/meson-log.txt && exit 1)
> > +- ninja -C _build_feat_disabled
> > +- ninja -C _build_feat_disabled test || (cat
> > _build_feat_disabled/meson-logs/testlog.txt && exit 1)
> >
> > -make-win:
> > +windows-autotools:
> >script:
> > - - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> > - - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> > make install)
> > - - NOCONFIGURE=yes ./autogen.sh
> > - - PYTHON=python3 mingw64-configure --enable-static
> > - - make -j4
> > +- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> > mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> > mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> > +- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr
> > &&
> > make install)
> > +- NOCONFIGURE=yes ./autogen.sh
> > +- PYTHON=python3 mingw64-configure --enable-static
> > +- make -j4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable
>
> From: Victor Toso
>
> To keep all dependencies together. Some (small) effort was made to
> distinguish what is necessary for Fedora and what is necessary for
> Windows builds in order to install only required packages when job is
> executing.
>
> Note that we are adding gettext, gettext-devel and glib2-devel
> explicit now on DEPS_COMMON. The reason is that, both Fedora and mingw
> builds require some tooling but for Fedora it gets installed
> indirectly while for Mingw, it doesn't and configure/build would fail.
>
> Signed-off-by: Victor Toso
Nice, do you have CI results? I suppose so.
> ---
> .gitlab-ci.yml | 28 +---
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index aab5f77..2627241 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -1,18 +1,24 @@
> image: fedora:latest
>
> variables:
> - DEPS: git libtool make python3 python3-six redhat-rpm-config
> -python3-pyparsing meson ninja-build zlib-devel openssl-devel
> -intltool gtk3-devel gtk-doc gobject-introspection-devel
> -cyrus-sasl-devel pulseaudio-libs-devel libjpeg-turbo-devel
> -libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
> -polkit-devel vala lz4-devel opus-devel
> -pixman-devel libcacard-devel celt051-devel libphodav-devel
> -usbutils usbredir-devel libusbx-devel libsoup-devel
> -json-glib-devel
> + DEPS_COMMON: git libtool make python3 python3-six redhat-rpm-config
> + python3-pyparsing meson ninja-build gtk-doc glib2-devel
> + gettext gettext-devel
Why not using the classic 4 space indentation?
> +
> + DEPS_FEDORA: zlib-devel openssl-devel intltool gtk3-devel
> + gobject-introspection-devel cyrus-sasl-devel
> + pulseaudio-libs-devel libjpeg-turbo-devel
> + libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
> + polkit-devel vala lz4-devel opus-devel pixman-devel
> + libcacard-devel celt051-devel libphodav-devel usbutils
> + usbredir-devel libusbx-devel libsoup-devel json-glib-devel
> +
> + DEPS_MINGW: mingw64-gcc mingw64-pkg-config mingw64-pixman mingw64-openssl
> + mingw64-gtk3 mingw64-json-glib mingw64-opus
> + mingw64-gstreamer1-plugins-base
> mingw64-gstreamer1-plugins-good
>
> before_script:
> - - dnf install -y $DEPS
> + - dnf install -y $DEPS_COMMON $DEPS_FEDORA
>- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
>- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
>
> @@ -47,7 +53,7 @@ fedora-meson:
>
> windows-autotools:
>script:
> -- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> +- dnf install -y $DEPS_COMMON $DEPS_MINGW
> - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> make install)
> - NOCONFIGURE=yes ./autogen.sh
> - PYTHON=python3 mingw64-configure --enable-static
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
>
> From: Victor Toso
>
> Group by target build instead of command. The focus of each job is to
> check any regression for given platform, using 'fedora'/'windows' and
> 'autotools'/'meson' seems more intuitive.
>
> By doing that we are grouping similar jobs together, this is
> intentional as we are reducing the amount of jobs that need to be run
> (together with the whole bootstrapping) without losing logs or tests.
>
> Some indentation takes place too, keeping it to 2 spaces as some other
> places in the code
>
Looking at the code I suppose you mean 4 not 2 in the last sentence.
> Signed-off-by: Victor Toso
> ---
> .gitlab-ci.yml | 62 --
> 1 file changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 74280e9..aab5f77 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -16,41 +16,39 @@ before_script:
>- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
>- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
>
> -makecheck:
> +fedora-autotools:
>script:
> - - ./autogen.sh --enable-static
> - - make -j4
> - - make check
> +# Run with default options
> +- ./autogen.sh --enable-static
> +- make -j4
> +- make check
> +# Run without features
> +- git clean -xfd
> +- ./autogen.sh --enable-static
> + --enable-lz4=no
> + --enable-webdav=no
> + --with-sasl=no
> + --with-coroutine=auto
> + --enable-pulse=no
> + --enable-smartcard=no
> + --enable-usbredir=no
> +- make -j4
> +- make check
>
> -makecheck-meson:
> +fedora-meson:
>script:
> - - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
> - - ninja -C build
> - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
> +- meson _build_default || (cat _build_default/meson-logs/meson-log.txt
> && exit 1)
Why the change to _build_default instead of build?
I suppose to group them but why the initial underscore?
Why not deleting the directory every time to reduce disk space?
> +- ninja -C _build_default
> +- ninja -C _build_default test || (cat
> _build_default/meson-logs/testlog.txt && exit 1)
>
> -makecheck_simple:
> - script:
> - - ./autogen.sh --enable-static
> ---enable-lz4=no
> ---enable-webdav=no
> ---with-sasl=no
> ---with-coroutine=auto
> ---enable-pulse=no
> ---enable-smartcard=no
> ---enable-usbredir=no
> - - make -j4
> - - make check
> -
> -makecheck_simple-meson:
> - script:
> - - meson build -Dauto_features=disabled || (cat
> build/meson-logs/meson-log.txt && exit 1)
> - - ninja -C build
> - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
> +- meson _build_feat_disabled -Dauto_features=disabled || (cat
> _build_feat_disabled/meson-logs/meson-log.txt && exit 1)
> +- ninja -C _build_feat_disabled
> +- ninja -C _build_feat_disabled test || (cat
> _build_feat_disabled/meson-logs/testlog.txt && exit 1)
>
> -make-win:
> +windows-autotools:
>script:
> - - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> - - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> make install)
> - - NOCONFIGURE=yes ./autogen.sh
> - - PYTHON=python3 mingw64-configure --enable-static
> - - make -j4
> +- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> +- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> make install)
> +- NOCONFIGURE=yes ./autogen.sh
> +- PYTHON=python3 mingw64-configure --enable-static
> +- make -j4
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
On Tue, Feb 12, 2019 at 3:18 PM Victor Toso wrote:
>
> From: Victor Toso
>
> Group by target build instead of command. The focus of each job is to
> check any regression for given platform, using 'fedora'/'windows' and
> 'autotools'/'meson' seems more intuitive.
>
> By doing that we are grouping similar jobs together, this is
> intentional as we are reducing the amount of jobs that need to be run
> (together with the whole bootstrapping) without losing logs or tests.
>
> Some indentation takes place too, keeping it to 2 spaces as some other
> places in the code
>
> Signed-off-by: Victor Toso
ack
> ---
> .gitlab-ci.yml | 62 --
> 1 file changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 74280e9..aab5f77 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -16,41 +16,39 @@ before_script:
>- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
>- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
>
> -makecheck:
> +fedora-autotools:
>script:
> - - ./autogen.sh --enable-static
> - - make -j4
> - - make check
> +# Run with default options
> +- ./autogen.sh --enable-static
> +- make -j4
> +- make check
> +# Run without features
> +- git clean -xfd
> +- ./autogen.sh --enable-static
> + --enable-lz4=no
> + --enable-webdav=no
> + --with-sasl=no
> + --with-coroutine=auto
> + --enable-pulse=no
> + --enable-smartcard=no
> + --enable-usbredir=no
> +- make -j4
> +- make check
>
> -makecheck-meson:
> +fedora-meson:
>script:
> - - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
> - - ninja -C build
> - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
> +- meson _build_default || (cat _build_default/meson-logs/meson-log.txt
> && exit 1)
> +- ninja -C _build_default
> +- ninja -C _build_default test || (cat
> _build_default/meson-logs/testlog.txt && exit 1)
>
> -makecheck_simple:
> - script:
> - - ./autogen.sh --enable-static
> ---enable-lz4=no
> ---enable-webdav=no
> ---with-sasl=no
> ---with-coroutine=auto
> ---enable-pulse=no
> ---enable-smartcard=no
> ---enable-usbredir=no
> - - make -j4
> - - make check
> -
> -makecheck_simple-meson:
> - script:
> - - meson build -Dauto_features=disabled || (cat
> build/meson-logs/meson-log.txt && exit 1)
> - - ninja -C build
> - - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
> +- meson _build_feat_disabled -Dauto_features=disabled || (cat
> _build_feat_disabled/meson-logs/meson-log.txt && exit 1)
> +- ninja -C _build_feat_disabled
> +- ninja -C _build_feat_disabled test || (cat
> _build_feat_disabled/meson-logs/testlog.txt && exit 1)
>
> -make-win:
> +windows-autotools:
>script:
> - - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> - - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> make install)
> - - NOCONFIGURE=yes ./autogen.sh
> - - PYTHON=python3 mingw64-configure --enable-static
> - - make -j4
> +- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
> mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
> mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
> +- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
> make install)
> +- NOCONFIGURE=yes ./autogen.sh
> +- PYTHON=python3 mingw64-configure --enable-static
> +- make -j4
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v1] Remove spicy flatpak from spice-gtk source code
On Tue, Feb 12, 2019 at 3:01 PM Victor Toso wrote:
>
> From: Victor Toso
>
> The usage of testing tool as flatkpak is discouraged by upstream
> developers, see comments from thread:
>
>
> https://lists.freedesktop.org/archives/spice-devel/2019-February/047877.html
>
> One might argue that keep this might be useful for documentation. For
> that, please refer to updated documentation from Flatpak and
> applications that have adopted Flatpak integration such as GNOME
> Boxes.
>
> Signed-off-by: Victor Toso
ack
> ---
> data/org.spicespace.spicy.json | 71 --
> 1 file changed, 71 deletions(-)
> delete mode 100644 data/org.spicespace.spicy.json
>
> diff --git a/data/org.spicespace.spicy.json b/data/org.spicespace.spicy.json
> deleted file mode 100644
> index 5b0dfbf..000
> --- a/data/org.spicespace.spicy.json
> +++ /dev/null
> @@ -1,71 +0,0 @@
> -{
> -"app-id": "org.spicespace.spicy",
> -"runtime": "org.gnome.Platform",
> -"runtime-version": "3.24",
> -"sdk": "org.gnome.Sdk",
> -"command": "spicy",
> -"tags": ["devel", "development", "nightly"],
> -"finish-args": [
> -"--share=ipc", "--socket=x11",
> -"--socket=wayland",
> -"--socket=pulseaudio",
> -"--share=network"
> -],
> -"modules": [
> -{
> -"name": "python-six",
> -"buildsystem": "simple",
> -"build-commands": [
> -"pip3 install --target=/app six-1.10.0-py2.py3-none-any.whl"
> -],
> -"sources": [
> -{
> - "type": "file",
> - "url":
> "https://pypi.python.org/packages/c8/0a/b6723e1bc4c516cb687841499455a8505b44607ab535be01091c0f24f079/six-1.10.0-py2.py3-none-any.whl#md5=3ab558cf5d4f7a72611d59a81a315dc8;,
> - "sha256":
> "0ff78c403d9bccf5a425a6d31a12aa6b47f1c21ca4dc2573a7e2f32a97335eb1"
> -}
> -]
> -},
> -{
> -"name": "pyparsing",
> -"buildsystem": "simple",
> -"build-commands": [
> -"pip3 install --target=/app
> pyparsing-2.0.3-py2.py3-none-any.whl"
> -],
> -"sources": [
> -{
> -"type": "file",
> -"url":
> "https://pypi.python.org/packages/8f/f4/3a70b5e5b865b1ec45fe48dc5a57cd4facb5c7bd80e5cb1255c362732e81/pyparsing-2.0.3-py2.py3-none-any.whl#md5=4c16ef222caea2e5de741ae503aae652;,
> -"sha256":
> "a9c896bab06dbf3759ad5fb63cfdb3777191e2c4ae640e6dd69ed37530f6ba56"
> -}
> -]
> -},
> -{
> -"builddir": true,
> -"name": "spice-protocol",
> -"sources": [
> -{
> -"type": "git",
> -"url":
> "https://gitlab.freedesktop.org/spice/spice-protocol.git;
> -}
> -]
> -},
> -{
> -"builddir": true,
> -"name": "spice-gtk",
> -"build-options": {
> -"env": {
> -"PYTHONPATH": "/app"
> -}
> -},
> -"config-opts": [ "--disable-vala",
> - "--disable-gtk-doc", "--enable-python-checks" ],
> -"sources": [
> -{
> -"type": "git",
> -"url":
> "https://gitlab.freedesktop.org/spice/spice-gtk.git;
> -}
> -]
> -}
> -]
> -}
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 4/5] gitlab-ci: add artifacts for logs and tests
From: Victor Toso
Much better than playing around with shell. Logs should live for week
since the build and CI will try to always upload them.
This patch also adds the logs for tests from builds with autotools
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aa67ed5..c68f9d7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -18,6 +18,12 @@ variables:
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
fedora-autotools:
+ artifacts:
+paths:
+ - tests/*.log
+when: always
+expire_in: 1 week
+
before_script:
- dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
@@ -42,6 +48,12 @@ fedora-autotools:
- make check
fedora-meson:
+ artifacts:
+paths:
+ - _build_*/meson-logs/*.txt
+when: always
+expire_in: 1 week
+
before_script:
- dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
@@ -49,13 +61,13 @@ fedora-meson:
- ninja -C _build_spice-protocol install
script:
-- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
+- meson _build_default
- ninja -C _build_default
-- ninja -C _build_default test || (cat
_build_default/meson-logs/testlog.txt && exit 1)
+- ninja -C _build_default test
-- meson _build_feat_disabled -Dauto_features=disabled || (cat
_build_feat_disabled/meson-logs/meson-log.txt && exit 1)
+- meson _build_feat_disabled -Dauto_features=disabled
- ninja -C _build_feat_disabled
-- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
+- ninja -C _build_feat_disabled test
windows-autotools:
before_script:
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 3/5] gitlab-ci: create before_script per job
From: Victor Toso
* On a windows job-build, we don't need to install Fedora dependencies.
This change makes only one dnf install be ran per job.
* On a meson build, we should build spice-protocol with meson too.
Moving this before_script rule to each job makes all of this clear.
So, this patch does change for fedora-meson, the build of
spice-protocol with meson and we don't (explicitly) install any of
$DEPS_FEDORA
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 2627241..aa67ed5 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -17,12 +17,12 @@ variables:
mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
-before_script:
- - dnf install -y $DEPS_COMMON $DEPS_FEDORA
- - git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- - (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
-
fedora-autotools:
+ before_script:
+- dnf install -y $DEPS_COMMON $DEPS_FEDORA
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
+
script:
# Run with default options
- ./autogen.sh --enable-static
@@ -42,6 +42,12 @@ fedora-autotools:
- make check
fedora-meson:
+ before_script:
+- dnf install -y $DEPS_COMMON $DEPS_FEDORA
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- meson spice-protocol _build_spice-protocol --prefix=/usr
+- ninja -C _build_spice-protocol install
+
script:
- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
- ninja -C _build_default
@@ -52,9 +58,12 @@ fedora-meson:
- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
windows-autotools:
- script:
+ before_script:
- dnf install -y $DEPS_COMMON $DEPS_MINGW
-- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- (cd spice-protocol && autoreconf -if && mingw64-configure --prefix=/usr
&& make install)
+
+ script:
- NOCONFIGURE=yes ./autogen.sh
- PYTHON=python3 mingw64-configure --enable-static
- make -j4
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable
From: Victor Toso
To keep all dependencies together. Some (small) effort was made to
distinguish what is necessary for Fedora and what is necessary for
Windows builds in order to install only required packages when job is
executing.
Note that we are adding gettext, gettext-devel and glib2-devel
explicit now on DEPS_COMMON. The reason is that, both Fedora and mingw
builds require some tooling but for Fedora it gets installed
indirectly while for Mingw, it doesn't and configure/build would fail.
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 28 +---
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aab5f77..2627241 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,18 +1,24 @@
image: fedora:latest
variables:
- DEPS: git libtool make python3 python3-six redhat-rpm-config
-python3-pyparsing meson ninja-build zlib-devel openssl-devel
-intltool gtk3-devel gtk-doc gobject-introspection-devel
-cyrus-sasl-devel pulseaudio-libs-devel libjpeg-turbo-devel
-libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
-polkit-devel vala lz4-devel opus-devel
-pixman-devel libcacard-devel celt051-devel libphodav-devel
-usbutils usbredir-devel libusbx-devel libsoup-devel
-json-glib-devel
+ DEPS_COMMON: git libtool make python3 python3-six redhat-rpm-config
+ python3-pyparsing meson ninja-build gtk-doc glib2-devel
+ gettext gettext-devel
+
+ DEPS_FEDORA: zlib-devel openssl-devel intltool gtk3-devel
+ gobject-introspection-devel cyrus-sasl-devel
+ pulseaudio-libs-devel libjpeg-turbo-devel
+ libacl-devel gstreamer1-devel gstreamer1-plugins-base-devel
+ polkit-devel vala lz4-devel opus-devel pixman-devel
+ libcacard-devel celt051-devel libphodav-devel usbutils
+ usbredir-devel libusbx-devel libsoup-devel json-glib-devel
+
+ DEPS_MINGW: mingw64-gcc mingw64-pkg-config mingw64-pixman mingw64-openssl
+ mingw64-gtk3 mingw64-json-glib mingw64-opus
+ mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
before_script:
- - dnf install -y $DEPS
+ - dnf install -y $DEPS_COMMON $DEPS_FEDORA
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
@@ -47,7 +53,7 @@ fedora-meson:
windows-autotools:
script:
-- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
+- dnf install -y $DEPS_COMMON $DEPS_MINGW
- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
- NOCONFIGURE=yes ./autogen.sh
- PYTHON=python3 mingw64-configure --enable-static
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs
From: Victor Toso
Group by target build instead of command. The focus of each job is to
check any regression for given platform, using 'fedora'/'windows' and
'autotools'/'meson' seems more intuitive.
By doing that we are grouping similar jobs together, this is
intentional as we are reducing the amount of jobs that need to be run
(together with the whole bootstrapping) without losing logs or tests.
Some indentation takes place too, keeping it to 2 spaces as some other
places in the code
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 62 --
1 file changed, 30 insertions(+), 32 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 74280e9..aab5f77 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -16,41 +16,39 @@ before_script:
- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
- (cd spice-protocol && ./autogen.sh --prefix=/usr && make install)
-makecheck:
+fedora-autotools:
script:
- - ./autogen.sh --enable-static
- - make -j4
- - make check
+# Run with default options
+- ./autogen.sh --enable-static
+- make -j4
+- make check
+# Run without features
+- git clean -xfd
+- ./autogen.sh --enable-static
+ --enable-lz4=no
+ --enable-webdav=no
+ --with-sasl=no
+ --with-coroutine=auto
+ --enable-pulse=no
+ --enable-smartcard=no
+ --enable-usbredir=no
+- make -j4
+- make check
-makecheck-meson:
+fedora-meson:
script:
- - meson build || (cat build/meson-logs/meson-log.txt && exit 1)
- - ninja -C build
- - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
+- meson _build_default || (cat _build_default/meson-logs/meson-log.txt &&
exit 1)
+- ninja -C _build_default
+- ninja -C _build_default test || (cat
_build_default/meson-logs/testlog.txt && exit 1)
-makecheck_simple:
- script:
- - ./autogen.sh --enable-static
---enable-lz4=no
---enable-webdav=no
---with-sasl=no
---with-coroutine=auto
---enable-pulse=no
---enable-smartcard=no
---enable-usbredir=no
- - make -j4
- - make check
-
-makecheck_simple-meson:
- script:
- - meson build -Dauto_features=disabled || (cat
build/meson-logs/meson-log.txt && exit 1)
- - ninja -C build
- - (cd build && meson test) || (cat build/meson-logs/testlog.txt && exit 1)
+- meson _build_feat_disabled -Dauto_features=disabled || (cat
_build_feat_disabled/meson-logs/meson-log.txt && exit 1)
+- ninja -C _build_feat_disabled
+- ninja -C _build_feat_disabled test || (cat
_build_feat_disabled/meson-logs/testlog.txt && exit 1)
-make-win:
+windows-autotools:
script:
- - dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
- - (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
- - NOCONFIGURE=yes ./autogen.sh
- - PYTHON=python3 mingw64-configure --enable-static
- - make -j4
+- dnf install -y mingw64-gcc mingw64-pkg-config mingw64-pixman
mingw64-openssl mingw64-gtk3 mingw64-json-glib mingw64-opus
mingw64-gstreamer1-plugins-base mingw64-gstreamer1-plugins-good
+- (cd spice-protocol && make clean && mingw64-configure --prefix=/usr &&
make install)
+- NOCONFIGURE=yes ./autogen.sh
+- PYTHON=python3 mingw64-configure --enable-static
+- make -j4
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 5/5] gitlab-ci: add mingw meson build
From: Victor Toso
To keep track of meson builds for windows too.
Note that mingw64-meson requires to be ran inside the folder,
different from common meson. So, some extra steps were done due that.
Signed-off-by: Victor Toso
---
.gitlab-ci.yml | 21 +
1 file changed, 21 insertions(+)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c68f9d7..71a0007 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -79,3 +79,24 @@ windows-autotools:
- NOCONFIGURE=yes ./autogen.sh
- PYTHON=python3 mingw64-configure --enable-static
- make -j4
+
+windows-meson:
+ artifacts:
+paths:
+ - _build_win64/meson-logs/*.txt
+ - spice-protocol/_build_spice-protocol/meson-logs/*.txt
+when: always
+expire_in: 1 week
+
+ before_script:
+- dnf install -y $DEPS_COMMON $DEPS_MINGW
+- git clone ${CI_REPOSITORY_URL/spice-gtk/spice-protocol}
+- mkdir spice-protocol/_build_spice-protocol && cd
spice-protocol/_build_spice-protocol
+- mingw64-meson --prefix=/usr
+- ninja install
+
+ script:
+- cd $CI_PROJECT_DIR
+- mkdir _build_win64 && cd _build_win64
+- mingw64-meson -Dgtk_doc=disabled
+- ninja install
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 0/5] gitlab-ci improvements
From: Victor Toso From v1, removed flatkpak fixes and autogeneration. Sending this as still seems to me that using artifacts, grouping some jobs and adding meson+mingw build is nice-to-have changes. You tell me. CI Run: https://gitlab.freedesktop.org/victortoso/spice-gtk/pipelines/18709 Victor Toso (5): gitlab-ci: group and rename jobs gitlab-ci: move windows dependencies to a variable gitlab-ci: create before_script per job gitlab-ci: add artifacts for logs and tests gitlab-ci: add mingw meson build .gitlab-ci.yml | 138 - 1 file changed, 92 insertions(+), 46 deletions(-) -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1] Remove spicy flatpak from spice-gtk source code
From: Victor Toso
The usage of testing tool as flatkpak is discouraged by upstream
developers, see comments from thread:
https://lists.freedesktop.org/archives/spice-devel/2019-February/047877.html
One might argue that keep this might be useful for documentation. For
that, please refer to updated documentation from Flatpak and
applications that have adopted Flatpak integration such as GNOME
Boxes.
Signed-off-by: Victor Toso
---
data/org.spicespace.spicy.json | 71 --
1 file changed, 71 deletions(-)
delete mode 100644 data/org.spicespace.spicy.json
diff --git a/data/org.spicespace.spicy.json b/data/org.spicespace.spicy.json
deleted file mode 100644
index 5b0dfbf..000
--- a/data/org.spicespace.spicy.json
+++ /dev/null
@@ -1,71 +0,0 @@
-{
-"app-id": "org.spicespace.spicy",
-"runtime": "org.gnome.Platform",
-"runtime-version": "3.24",
-"sdk": "org.gnome.Sdk",
-"command": "spicy",
-"tags": ["devel", "development", "nightly"],
-"finish-args": [
-"--share=ipc", "--socket=x11",
-"--socket=wayland",
-"--socket=pulseaudio",
-"--share=network"
-],
-"modules": [
-{
-"name": "python-six",
-"buildsystem": "simple",
-"build-commands": [
-"pip3 install --target=/app six-1.10.0-py2.py3-none-any.whl"
-],
-"sources": [
-{
- "type": "file",
- "url":
"https://pypi.python.org/packages/c8/0a/b6723e1bc4c516cb687841499455a8505b44607ab535be01091c0f24f079/six-1.10.0-py2.py3-none-any.whl#md5=3ab558cf5d4f7a72611d59a81a315dc8;,
- "sha256":
"0ff78c403d9bccf5a425a6d31a12aa6b47f1c21ca4dc2573a7e2f32a97335eb1"
-}
-]
-},
-{
-"name": "pyparsing",
-"buildsystem": "simple",
-"build-commands": [
-"pip3 install --target=/app
pyparsing-2.0.3-py2.py3-none-any.whl"
-],
-"sources": [
-{
-"type": "file",
-"url":
"https://pypi.python.org/packages/8f/f4/3a70b5e5b865b1ec45fe48dc5a57cd4facb5c7bd80e5cb1255c362732e81/pyparsing-2.0.3-py2.py3-none-any.whl#md5=4c16ef222caea2e5de741ae503aae652;,
-"sha256":
"a9c896bab06dbf3759ad5fb63cfdb3777191e2c4ae640e6dd69ed37530f6ba56"
-}
-]
-},
-{
-"builddir": true,
-"name": "spice-protocol",
-"sources": [
-{
-"type": "git",
-"url":
"https://gitlab.freedesktop.org/spice/spice-protocol.git;
-}
-]
-},
-{
-"builddir": true,
-"name": "spice-gtk",
-"build-options": {
-"env": {
-"PYTHONPATH": "/app"
-}
-},
-"config-opts": [ "--disable-vala",
- "--disable-gtk-doc", "--enable-python-checks" ],
-"sources": [
-{
-"type": "git",
-"url": "https://gitlab.freedesktop.org/spice/spice-gtk.git;
-}
-]
-}
-]
-}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-server v2 1/4] smartcard: Remove unused smartcard_get_vsc_msg_item argument
>
> Signed-off-by: Christophe Fergeau
Acked the series.
Frediano
> ---
> server/smartcard.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/server/smartcard.c b/server/smartcard.c
> index ff680d8a5..21dc8de5a 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -103,7 +103,7 @@ typedef struct RedMsgItem {
> VSCMsgHeader* vheader;
> } RedMsgItem;
>
> -static RedMsgItem *smartcard_get_vsc_msg_item(RedChannelClient *rcc,
> VSCMsgHeader *vheader);
> +static RedMsgItem *smartcard_get_vsc_msg_item(VSCMsgHeader *vheader);
>
> static struct Readers {
> uint32_t num;
> @@ -217,8 +217,7 @@ RedMsgItem
> *smartcard_char_device_on_message_from_device(RedCharDeviceSmartcard
> /* We patch the reader_id, since the device only knows about itself,
> and
> * we know about the sum of readers. */
> sent_header->reader_id = dev->priv->reader_id;
> -return
> smartcard_get_vsc_msg_item(RED_CHANNEL_CLIENT(dev->priv->scc),
> - sent_header);
> +return smartcard_get_vsc_msg_item(sent_header);
> }
> return NULL;
> }
> @@ -457,8 +456,7 @@ static void smartcard_free_vsc_msg_item(RedPipeItem
> *base)
> g_free(item);
> }
>
> -static RedMsgItem *smartcard_get_vsc_msg_item(RedChannelClient *rcc,
> - VSCMsgHeader *vheader)
> +static RedMsgItem *smartcard_get_vsc_msg_item(VSCMsgHeader *vheader)
> {
> RedMsgItem *msg_item = g_new0(RedMsgItem, 1);
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk [rfc] 1/2] gtk-session: introduce clipboard-managers property
Hi,
On Tue, Jan 15, 2019 at 5:11 PM Victor Toso wrote:
>
> From: Victor Toso
>
> SpiceGtkSession::allow-clipboard-managers property is introduced to
> enable other applications in the Client OS to set or fetch clipboard
> data from a spice-gtk-session that is under keyboard-grab, which is
> expected to be in use by the user.
>
> This option is disabled by default as to avoid:
> 1) Another application in the client to fetch clipboard data while the
>user is interacting with the remote machine;
You're implementing this by leaving the GtkSelectionData empty upon
GtkClipboardGetFunc.
This seems weird to me. By grabbing the clipboard, spice-gtk proclaims
it can provide clipboard data, but if another app asks for this data,
spice-gtk provides none.
It naturally depends on the implementation of the given app, but I
think that in the case of a clipboard manager, user could see empty
entries in the clipboard history, which is imho unwanted. (haven't
tested)
If spice-gtk is not willing to give the data to other apps, it
shouldn't advertise it in the first place, I think.
So I would rather implement it by delaying the grab the same way you
do it in case 2) below.
> 2) Another application to set clipboard data *and* send that to the
>remote machine while the user is *not* interacting with the remote
>machine (without keyboard grab)
I see that you're trying to prevent apps in guest from eavesdropping
on the client clipboard while the user is not interacting with the
remote machine.
But I think there's another point of view: if the given app grabbed
the clipboard, it can also be interpreted as that the app decided to
share its data with other apps. And in that case, I don't see why
spice-gtk should impede the sharing. Note, that e.g. obscured
passwords in browsers cannot be copied in the clipboard (this is an
example of the case when app doesn't want to share its data).
Apart from that, there isn't really any good way to clear the
clipboard afaik, unless you store something new in it. Doesn't seem
like something a lot of users would do. So the sensitive data would
get leaked anyway the moment you click into the remote machine's
window.
So I'm rather inclined to agree with the opinion that this should be
enforced by the desktop, not us.
Cheers,
Jakub
>
> This patch disables (1) and following patch disables (2) if
> allow-clipboard-managers is disabled which is set to be default.
>
> This patch also fixes some indentation found in spice-gtk-session.c
>
> Signed-off-by: Victor Toso
> ---
> src/spice-gtk-session.c | 51 +
> tools/spicy.c | 6 +
> 2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index adc72a2..7391e6a 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
> gbooleanclip_hasdata[CLIPBOARD_LAST];
> gbooleanclip_grabbed[CLIPBOARD_LAST];
> gbooleanclipboard_by_guest[CLIPBOARD_LAST];
> +gbooleanback_from_focus_out;
> /* auto-usbredir related */
> gbooleanauto_usbredir_enable;
> int auto_usbredir_reqs;
> @@ -66,6 +67,7 @@ struct _SpiceGtkSessionPrivate {
> gbooleankeyboard_has_focus;
> gbooleanmouse_has_pointer;
> gbooleansync_modifiers;
> +gbooleanallow_clipboard_managers;
> };
>
> /**
> @@ -114,6 +116,7 @@ enum {
> PROP_AUTO_USBREDIR,
> PROP_POINTER_GRABBED,
> PROP_SYNC_MODIFIERS,
> +PROP_ALLOW_CLIPBOARD_MANAGER,
> };
>
> static guint32 get_keyboard_lock_modifiers(void)
> @@ -287,6 +290,9 @@ static void spice_gtk_session_get_property(GObject
> *gobject,
> case PROP_SYNC_MODIFIERS:
> g_value_set_boolean(value, s->sync_modifiers);
> break;
> +case PROP_ALLOW_CLIPBOARD_MANAGER:
> +g_value_set_boolean(value, s->allow_clipboard_managers);
> +break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> @@ -337,6 +343,9 @@ static void spice_gtk_session_set_property(GObject
> *gobject,
> case PROP_SYNC_MODIFIERS:
> s->sync_modifiers = g_value_get_boolean(value);
> break;
> +case PROP_ALLOW_CLIPBOARD_MANAGER:
> +s->allow_clipboard_managers = g_value_get_boolean(value);
> +break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> @@ -441,6 +450,23 @@ static void
> spice_gtk_session_class_init(SpiceGtkSessionClass *klass)
>G_PARAM_READWRITE |
>G_PARAM_CONSTRUCT |
>G_PARAM_STATIC_STRINGS));
> +/**
> + * SpiceGtkSession:allow-clipboard-managers
> + *
> + * Allow
Re: [Spice-devel] [PATCH spice-gtk 09/12] doc: fix a few links
>
> From: Marc-André Lureau
>
> Signed-off-by: Marc-André Lureau
Acked-by: Frediano Ziglio
> ---
> src/channel-display.c | 2 +-
> src/channel-main.h| 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/channel-display.c b/src/channel-display.c
> index ae949eb..fd072a9 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -436,7 +436,7 @@ static void
> spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
> * @width: width
> * @height: height
> *
> - * The #SpiceDisplayChannel::draw signal is emitted when the
> + * The #SpiceDisplayChannel::gl-draw signal is emitted when the
> * rectangular region x/y/w/h of the GL scanout is updated and
> * must be drawn. When the draw is finished, you must call
> * spice_display_gl_draw_done() in order to release the GL
> diff --git a/src/channel-main.h b/src/channel-main.h
> index fd7a3bb..3c527d6 100644
> --- a/src/channel-main.h
> +++ b/src/channel-main.h
> @@ -53,8 +53,8 @@ struct _SpiceMainChannel {
> /**
> * SpiceMainChannelClass:
> * @parent_class: Parent class.
> - * @mouse_update: Signal class handler for the
> #SpiceMainChannel::mouse-update signal.
> - * @agent_update: Signal class handler for the
> #SpiceMainChannel::agent-update signal.
> + * @mouse_update: Signal class handler for the
> #SpiceMainChannel::main-mouse-update signal.
> + * @agent_update: Signal class handler for the
> #SpiceMainChannel::main-agent-update signal.
> *
> * Class structure for #SpiceMainChannel.
> */
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 03/12] meson: fix po generation
> From: Marc-André Lureau
>
> Use glib preset (from meson v0.37) to catch all our translatable
> strings and use good default settings.
>
> While at it, remove the needless directory argument.
>
> Signed-off-by: Marc-André Lureau
With or without this patch Meson seems to do nothing in the po/
directory, .gmo files are not generated.
I cannot see any warning in Meson output about i18n.
Frediano
> ---
> po/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/po/meson.build b/po/meson.build
> index 60c27a7..fb3c395 100644
> --- a/po/meson.build
> +++ b/po/meson.build
> @@ -1,3 +1,3 @@
> i18n = import('i18n')
> i18n.gettext(meson.project_name(),
> - args : '--directory=@0@'.format(meson.source_root()))
> + preset : 'glib')
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 spice-gtk] Update the codeflow description comment
> On Mon, 2019-02-11 at 16:14 +0200, Snir Sheriber wrote: > > --- > > src/channel-display-gst.c | 39 ++--- > > -- > > 1 file changed, 22 insertions(+), 17 deletions(-) > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > index 4272ade..c63592b 100644 > > --- a/src/channel-display-gst.c > > +++ b/src/channel-display-gst.c > > @@ -580,34 +580,39 @@ static void > > spice_gst_decoder_destroy(VideoDecoder *video_decoder) > > /* spice_gst_decoder_queue_frame() queues the SpiceFrame for > > decoding and > > * displaying. The steps it goes through are as follows: > > * > > - * 1) A SpiceGstFrame is created to keep track of SpiceFrame and > > some additional > > - *metadata. The SpiceGstFrame is then pushed to the > > decoding_queue. > > - * 2) frame->data, which contains the compressed frame data, is > > reffed and > > - *wrapped in a GstBuffer which is pushed to the GStreamer > > pipeline for > > - *decoding. > > - * 3) As soon as the GStreamer pipeline no longer needs the > > compressed frame it > > - *will call frame->unref_data() to free it. > > + * 1) frame->data, which contains the compressed frame data, is > > wrapped in a GstBuffer > > + *(encoded_buffer) which owns the SpiceFrame. > > + * 2) A SpiceGstFrame is created to keep track of SpiceFrame > > (encoded_frame), and some > > + *additional metadata. The encoded_buffer is referenced and the > > SpiceGstFrame is then > > + *pushed into the decoding_queue. > > * > > * If GstVideoOverlay is used (window handle was obtained > > successfully at the widget): > > - * 4) Decompressed frames will be renderd to widget directly from > > gstreamer's pipeline > > - * using some gstreamer sink plugin which implements the > > GstVideoOverlay interface > > + * 3) Decompressed frames will be rendered to widget directly from > > GStreamer's pipeline > > "to *the* widget" sounds more correct. Otherwise I'd say the rest of > this patch looks fine. I only reviewed it for language, though. I > didn't check that the description actually matches the code. > > Reviewed-by: Jonathon Jongsma > > Description is fine, Acked-by: Frediano Ziglio Frediano > > > + * using some GStreamer sink plugin which implements the > > GstVideoOverlay interface > > * (last step). > > + * 4) As soon as GStreamer's pipeline no longer needs the > > compressed frame it will > > + * unref the encoded_buffer > > + * 5) Once a decoded buffer arrives to the sink (notified by probe > > event) we will pop > > + * its matching SpiceGstFrame from the decoding_queue and free > > it using > > + * free_gst_frame, this will also unref the encoded_buffer > > which will allow > > + * GStreamer to call spice_frame_free and free its > > encoded_frame. > > * > > * Otherwise appsink is used: > > - * 4) Once the decompressed frame is available the GStreamer > > pipeline calls > > + * 3) Once the decompressed frame is available the GStreamer > > pipeline calls > > * new_sample() in the GStreamer thread. > > - * 5) new_sample() then matches the decompressed frame to a > > SpiceGstFrame from > > + * 4) new_sample() then matches the decompressed frame to a > > SpiceGstFrame from > > * the decoding queue using the GStreamer timestamp information > > to deal with > > * dropped frames. The SpiceGstFrame is popped from the > > decoding_queue. > > - * 6) new_sample() then attaches the decompressed frame to the > > SpiceGstFrame, > > + * 5) new_sample() then attaches the decompressed frame to the > > SpiceGstFrame, > > * set into display_frame and calls schedule_frame(). > > - * 7) schedule_frame() then uses gstframe->frame->mm_time to > > arrange for > > + * 6) schedule_frame() then uses gstframe->encoded_frame->mm_time > > to arrange for > > * display_frame() to be called, in the main thread, at the > > right time for > > * the next frame. > > - * 8) display_frame() use SpiceGstFrame from display_frame and > > - * calls stream_display_frame(). > > - * 9) display_frame() then frees the SpiceGstFrame, which frees > > the SpiceFrame > > - * and decompressed frame with it. > > + * 7) display_frame() uses SpiceGstFrame from display_frame and > > calls > > + * stream_display_frame(). > > + * 8) display_frame() then calls to free_gst_frame to free the > > SpiceGstFrame and > > + * unref the encoded_buffer which allows GStreamer to call > > spice_frame_free > > + * and free its encoded_frame. > > */ > > static gboolean spice_gst_decoder_queue_frame(VideoDecoder > > *video_decoder, > >SpiceFrame *frame, int > > latency) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message
> Untested, but looks fine.
>
> Acked-by: Jonathon Jongsma
>
It's partially tested by following patch. Partially as the new function is
tested but the old function to send the message is not but is changed in
this test.
But I can see data sent to the guest (so I tested that part manually).
Should the verb be "marshall" or "marshal" ? I think the second, so
I should rename the function.
Frediano
>
>
> On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote:
> > Instead of scanning the monitor twice (one to compute the size
> > and another to build the message) use a single function to
> > marshall the message.
> > This also fixes big endian machines (which are not supported).
> > Marshall function is exported to make easier to test (see following
> > patch).
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > Also the reds_marshall_device_display_info function will be
> > potentially used to separate VDIPort code.
> > ---
> > server/reds.c | 143 ++
> >
> > server/reds.h | 3 ++
> > 2 files changed, 66 insertions(+), 80 deletions(-)
> >
> > diff --git a/server/reds.c b/server/reds.c
> > index 86f020a87..c16655adf 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -897,78 +897,33 @@ static RedPipeItem
> > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > return NULL;
> > }
> >
> > -void reds_send_device_display_info(RedsState *reds)
> > +void reds_marshall_device_display_info(RedsState *reds,
> > SpiceMarshaller *m)
> > {
> > -if (!reds->agent_dev->priv->agent_attached) {
> > -return;
> > -}
> > -g_debug("Sending device display info to the agent:");
> > -
> > QXLInstance *qxl;
> > RedCharDevice *dev;
> >
> > -size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > -
> > -// size for the qxl device info
> > -FOREACH_QXL_INSTANCE(reds, qxl) {
> > -message_size +=
> > -(sizeof(VDAgentDeviceDisplayInfo) +
> > strlen(red_qxl_get_device_address(qxl)) + 1) *
> > -red_qxl_get_monitors_count(qxl);
> > -}
> > -
> > -// size for the stream device info
> > -GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> > -if (IS_STREAM_DEVICE(dev)) {
> > -size_t device_address_len =
> > -strlen(stream_device_get_device_display_info(STREAM_
> > DEVICE(dev))->device_address);
> > -
> > -if (device_address_len == 0) {
> > -// the device info wasn't set (yet), don't send it
> > -continue;
> > -}
> > -
> > -message_size += (sizeof(VDAgentDeviceDisplayInfo) +
> > device_address_len + 1);
> > -}
> > -}
> > -
> > -RedCharDeviceWriteBuffer *char_dev_buf =
> > vdagent_new_write_buffer(reds->agent_dev,
> > - VD_AGENT_GRAPHICS_DEVICE_IN
> > FO,
> > - message_size,
> > - true);
> > -
> > -if (!char_dev_buf) {
> > -reds->pending_device_display_info_message = true;
> > -return;
> > -}
> > -
> > -VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf-
> > >buf;
> > -VDAgentGraphicsDeviceInfo *graphics_device_info = _buf-
> > >u.graphics_device_info;
> > -graphics_device_info->count = 0;
> > -
> > -VDAgentDeviceDisplayInfo *device_display_info =
> > graphics_device_info->display_info;
> > +uint32_t device_count = 0;
> > +void *device_count_ptr = spice_marshaller_add_uint32(m,
> > device_count);
> >
> > // add the qxl devices to the message
> > FOREACH_QXL_INSTANCE(reds, qxl) {
> > +const char *const device_address =
> > red_qxl_get_device_address(qxl);
> > +const size_t device_address_len = strlen(device_address) +
> > 1;
> > +if (device_address_len == 1) {
> > +continue;
> > +}
> > for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i)
> > {
> > -device_display_info->channel_id = qxl->id;
> > -device_display_info->monitor_id = i;
> > -device_display_info->device_display_id =
> > red_qxl_get_device_display_ids(qxl)[i];
> > -
> > -strcpy((char*) device_display_info->device_address,
> > red_qxl_get_device_address(qxl));
> > -
> > -device_display_info->device_address_len =
> > -strlen((char*) device_display_info->device_address)
> > + 1;
> > -
> > -g_debug(" (qxl)channel_id: %u monitor_id: %u,
> > device_address: %s, device_display_id: %u",
> > -device_display_info->channel_id,
> > -device_display_info->monitor_id,
> > -device_display_info->device_address,
> > -device_display_info->device_display_id);
> > -
> > -device_display_info = (VDAgentDeviceDisplayInfo*)
> > ((char*) device_display_info +
> > -
Re: [Spice-devel] [spice-gtk v1 00/10] Flatpak + CI
Hey, On Mon, Feb 11, 2019 at 11:51:54PM +0100, Marc-André Lureau wrote: > Hi > > On Mon, Feb 11, 2019 at 6:12 PM Christophe Fergeau > wrote: > > I think the main objection is with making spicy too easy to install (and > > to upgrade). Once we ask someone to test a spicy flatpak and it works > > for them, we don't want them to stick to it, start requesting for > > flathub availability so that it gets regularly updated, and for this one > > small feature that would make spicy a perfect fit for them (which is why > > in the first place Marc-André has been trying to discourage use of > > spicy). > > > > Indeed. So far it is there as an "example": > > commit 64a0eeab8ddd2ca6b2d3b57b7f46e99877bfab7e > Author: Pavel Grunt > Date: Fri Jul 21 11:02:57 2017 +0200 > > Add flatpak builder manifest file for spicy > > To give an example for creating flatpaks depending on spice-gtk > > > Tbh, I think we should remove the flatpak from spice-gtk source tree. > It doesn't make much sense to have it included imho, unless we have a > good reason to build it on a regular basis, which imho is not > something we need as a library or even a testing client. Is there a repository of flatpak build snippets these days? If not, I think it can be useful to document a canonical way of building spice-gtk in a flatpak, rather than having every application build spice-gtk in its own way. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
