Re: [Spice-devel] [spice-common v4 3/7] log: Use glib for logging
I realize I'm rather late to the party, but this change causes troubles
with Xspice.
That is, Xorg, on startup, closes stdout.
Since the g_log_default_handler writes to stdout for DEBUG and INFO
messages, those messages never show up anywhere.
The attached patch 'fixes' it. I'm a bit hesitant to pursue this
approach; I do not know if using stdout will have a negative side effect
on the Xorg server.
Anyone else have insight? I'll more formally submit this if I don't
hear anything back.
Cheers,
Jeremy
diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index b2b31ff..f9ddba1 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -28,6 +28,8 @@
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
+#include
+
#include "qxl.h"
#include "qxl_option_helpers.h"
@@ -39,6 +41,9 @@ SpiceServer *xspice_get_spice_server(void)
{
static SpiceServer *spice_server;
if (!spice_server) {
+/* Xorg closes stdout on us. We reopen it to point at stderr, so that
+ g_log_default_handler will send it's output somewhere useful */
+dup2(2, 1);
spice_server = spice_server_new();
}
return spice_server;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [qxl 0/5] Xspice audio playback fixes
Hey Francois, On 03/18/2016 09:31 AM, Francois Gouget wrote: > > While testing the video streaming I ran into a number of issues caused > by Xspice's audio playback. So I investigated those and found fixes > which gives this patch series. > > This patch set is totally independent from the video streaming series. > That said it does help test video streaming special cases as described > below: I had one comment on a comment in patch 5, but I've tested this; it all works, and the patches all also look correct to me. So modulo a comment on patch 5, ACK series. Cheers, Jeremy > > > Francois Gouget (5): > spiceqxl_audio: Let the audio play when no client is connected > - This allows testing what happens to video streaming when there is > no client. > > spiceqxl_audio: Only condense the fifo list when one has been closed > > spiceqxl_audio: Fix a race condition in the audio playback > - By preventing the audio from getting stuck after a while this > allows testing the video streaming for longer periods. > > spiceqxl_audio: Only start the playback channel when fifos are present > - This allows testing the video streaming independently from the > audio, and thus to determine which side is causing trouble. > > spiceqxl_audio: Stop the playback channel if there is nothing to play > > src/spiceqxl_audio.c | 174 > --- > 1 file changed, 109 insertions(+), 65 deletions(-) > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [qxl 5/5] spiceqxl_audio: Stop the playback channel if there is nothing to play
Hey Francois,
> @@ -361,14 +366,23 @@ static void start_watching(qxl_screen_t *qxl)
> }
> }
>
> +/* a helper for read_from_fifos() */
> static void wall_ticker(void *opaque)
> {
> qxl_screen_t *qxl = opaque;
> struct audio_data *data = qxl->playback_opaque;
>
> -data->wall_timer_live = 0;
> -
> -read_from_fifos(-1, 0, qxl);
> +if (data->wall_timer_type == IDLE_MS) {
> +/* There is no open fifo anymore */
I think this comment is not necessarily correct. The fifos could still
be open, just not producing audio data, right?
Cheers,
Jeremy
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] spice-html5 keyboard layout fix proposal
Hi Eric, This is great; thanks for taking the time to put this together. Could I ask a few favors, though? This version did not apply to the git tip for me. I think a few corrections Christophe made recently keep your utils.js change from applying. Additionally, if you could keep the spice.html change out of this patch, that would help in reviewing. A brief overview suggests your changes are promising. However, I did find a fair number of keys that no longer work (.=; q and a are reversed, there are others). I'm also troubled that you've taken out some of the Firefox specific scan codes - did yuo try multiple browsers as you worked on this? And can I presume that this code as written works reasonably well for you? Cheers, Jeremy On 03/17/2016 11:50 AM, Eric Grammatico wrote: > Gentle(wo)men, > > I have spent time to fix the keyboard layout in spice-html5. I went to > the conclusion the keyboard layout cannot be configured on client > (html5) side as Xorg receives key code it interpretes. What ever the key > face the user has typed (A or Q) Xorg will map to the character > depending of its configuration. > > The drawback was on my setup Xspice ignores my keyboard config and the > only way I found to force my keyboard layout was to launch setxkbmap > from .xinitrc. > > I kept the same structure around utils.js, atKeynames.js and inputs.js. > In atKeynames I mapped the Xorg symbols defined in > /usr/share/X11/xkb/symbols. This way, all the non changing keys (like > ESCAPE) through the different layouts are well defined and the others > are map to symbols like AD01. In some how it's kept readable... > > Then I have updated utils.js and inputs.js in consequence. > > Please find attached my diff file as a proposal. Tests and comments are > very wellcome. > > Thanks and regards, > > - > _/) Eric Grammatico. > > > ___ > 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] [PATCH v3 2/2] Lower gtk+ requirement to 3.10
Hi, > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c > index 1b3cd07..a9bea52 100644 > --- a/src/spice-widget-egl.c > +++ b/src/spice-widget-egl.c > @@ -29,7 +29,9 @@ > #include > > #include > +#if GTK_CHECK_VERSION(3,16,0) > #include > +#endif This breaks builds on Debian, which has 3.14. You get a failure on line 208 because GDK_IS_WAYLAND_DISPLAY is not defined. I was able to get it to compile by changing this particular #ifdef to just be #ifdef GDK_WINDOWING_WAYLAND It's not clear if that's right, or if it would be better to make all use of Wayland conditional on 3,16. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk] Include gdk/gdkwayland.h anytime GDK_WINDOWING_WAYLAND is defined.
A specific GTK version check is not appropriate for this particular include. This fixes compilation on Debian Jessie. Signed-off-by: Jeremy White --- src/spice-widget-egl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c index a9bea52..883983e 100644 --- a/src/spice-widget-egl.c +++ b/src/spice-widget-egl.c @@ -29,7 +29,7 @@ #include #include -#if GTK_CHECK_VERSION(3,16,0) +#ifdef GDK_WINDOWING_WAYLAND #include #endif -- 2.1.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/2] Lower gtk+ requirement to 3.10
On 03/22/2016 08:33 AM, Marc-André Lureau wrote: > Hi > > - Original Message - >> Hi, >> >>> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c >>> index 1b3cd07..a9bea52 100644 >>> --- a/src/spice-widget-egl.c >>> +++ b/src/spice-widget-egl.c >>> @@ -29,7 +29,9 @@ >>> #include >>> >>> #include >>> +#if GTK_CHECK_VERSION(3,16,0) >>> #include >>> +#endif >> >> This breaks builds on Debian, which has 3.14. You get a failure on line >> 208 because GDK_IS_WAYLAND_DISPLAY is not defined. >> >> I was able to get it to compile by changing this particular #ifdef to >> just be >> #ifdef GDK_WINDOWING_WAYLAND > > I think that makes sense. It's defined in gdkconfig.h which is included by > gdk.h. As long as it compiles on 3.10, it is good enough. Would you like to > send a patch? Patch sent. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common v4 3/7] log: Use glib for logging
On 03/24/2016 10:20 AM, Christophe Fergeau wrote: > Hey, > > Sorry, hadn't noticed this message earlier. > > On Wed, Mar 09, 2016 at 01:43:14PM -0600, Jeremy White wrote: >> I realize I'm rather late to the party, but this change causes >> troubles with Xspice. >> >> That is, Xorg, on startup, closes stdout. >> >> Since the g_log_default_handler writes to stdout for DEBUG and >> INFO messages, those messages never show up anywhere. > > Ah, and this worked before because all messages wer output to > stderr > >> >> The attached patch 'fixes' it. I'm a bit hesitant to pursue >> this approach; I do not know if using stdout will have a negative >> side effect on the Xorg server. >> >> Anyone else have insight? I'll more formally submit this if I >> don't hear anything back. > > An alternative would be to use g_log_set_default_handler() The current log.c/spice_logger() code invokes g_log_default_handler () directly, so setting our own default handler does no good. That would work if we did not set up spice_logger() as a handler; it's not entirely clear to me why we need to do that. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] RFC: XSpice shift to an 'x11spice' approach
Hi folks, As you know, I've done a lot of work on XSpice over the past few years. I plan to take a new approach by following the model of the excellent 'x11vnc' client. That is, instead of having our own X server and capturing the output, I plan to instead use an xdummy style server with a frame buffer which then runs an x11spice client to push the display via the Spice protocol. This should have a number of benefits. First, it should create a new capability - the ability to share an existing desktop via Spice. Second, it should allow the x11spice process to run entirely as a non privileged user. This should simplify some of the existing Xspice management headaches. (Full candor note: it simplifies some of the private use cases I have for Xspice, where security and process isolation is vital). Third, it should allow for some simplification by removing the need to have our own X server. That simplification does come with a cost - we now have the expanded complexity of intercepting X events and managing that at an application level, instead of at a driver level, so that's arguably a wash. Finally, I'm hoping that I may be able to do some thoughtful analysis of the approach taken by Xpra, and either incorporate Xpra directly, or at least leave a path open for it, so that we could have 'rootless' or seamless applications as an option. What do others think? What am I over looking? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: XSpice shift to an 'x11spice' approach
On 03/29/2016 05:43 AM, Francois Gouget wrote: > On Mon, 28 Mar 2016, Jeremy White wrote: > [...] > >> Hi folks, >> >> As you know, I've done a lot of work on XSpice over the past few years. >> I plan to take a new approach by following the model of the excellent >> 'x11vnc' client. > [...] >> What do others think? What am I over looking? > > One of the big features of Spice is being able to transport the audio > channel. However it seems like this relies on being able to set PULSE* > variables in the environment of the processes running in the X session. > That would not be possible when connecting to an existing X11 session. > Would alternative mechanism be able to compensate? I think Pulse provides enough tools that it should be possible to come up with a way to transport audio. Of course, nothing keeps you from completely replicating the current functionality; with an x11spice you would still be able to set up a dummy xorg and control it's start up environment. > > Spice can also remote other resources but I don't have a sense of how > much those would be impacted. > > Also, would that mean that the Xspice script would become a wrapper for > starting an X session and the related x11spice client? I don't think I would change Xspice; I think I would just add a new 'x11spice' utility. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: XSpice shift to an 'x11spice' approach
On 03/30/2016 04:57 AM, Pedro Francisco wrote: > Sending to list as well, this time. > > 2016-03-28 15:19 GMT+01:00 Jeremy White : >> Hi folks, >> >> As you know, I've done a lot of work on XSpice over the past few years. >> I plan to take a new approach by following the model of the excellent >> 'x11vnc' client. > > > Tell me, performance-wise, is there any different between a real X11 > on real hardware and a X11 on XvFb or equivalent? Performance-wise, > will one be hardware-accelerated and the other software-render only? > Well, yes, there is a difference between X11 running with a video card with a GPU and X11 using the dummy driver and software rendering; the former has a GPU to accelerate draws and generally benefits from glx and opengl functionality. I'm not sure I follow the relevance to this thread, though. Both Xspice and x11spice would, by default, use an X frame buffer sort of driver. It is the case, though, that the benefit that x11spice could be run against a main session probably does mean that you'd get the benefit of the GPU acceleration of that session. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-html5 1/2] filexfer: Remove wrongly placed semicolon
On 05/04/2016 07:57 AM, Pavel Grunt wrote:
> Spotted by coverity
Tested, acked, and pushed.
Thanks,
Jeremy
> ---
> filexfer.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/filexfer.js b/filexfer.js
> index beabfd8..d850db2 100644
> --- a/filexfer.js
> +++ b/filexfer.js
> @@ -81,7 +81,7 @@ function handle_file_drop(e)
> e.preventDefault();
> for (var i = files.length - 1; i >= 0; i--)
> {
> -if (files[i].type); // do not copy a directory
> +if (files[i].type) // do not copy a directory
> sc.file_xfer_start(files[i]);
> }
>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH xf86-video-qxl] Correct a long standing led state bug in XSpice.
The CtrlProc for our keyboard driver incorrectly mapped
the device private to a SpiceKbd* intead of to a InputInfoPtr.
That resulted in led state being written into the driver name
for our driver structure, instead of into the led state.
That, in turn, led to a cool bug where if you pressed caps lock,
the two second sync timer in the spice server would cause it to
attempt to correct the state by pressing caps lock to get the
states to match. Since the states will never match, the caps
lock effectively cycles on and off every two seconds.
Signed-off-by: Jeremy White
---
src/spiceqxl_inputs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/spiceqxl_inputs.c b/src/spiceqxl_inputs.c
index b39eeae..5625309 100644
--- a/src/spiceqxl_inputs.c
+++ b/src/spiceqxl_inputs.c
@@ -138,9 +138,11 @@ static void xspice_keyboard_control(DeviceIntPtr device,
KeybdCtrl *ctrl)
};
XSpiceKbd *kbd;
+InputInfoPtr pInfo;
int i;
-kbd = device->public.devicePrivate;
+pInfo = device->public.devicePrivate;
+kbd = pInfo->private;
kbd->ledstate = 0;
for (i = 0; i < ArrayLength(bits); i++) {
if (ctrl->leds & bits[i].xbit) {
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH xf86-video-qxl] Correct a long standing led state bug in XSpice.
Signed-off-by: Jeremy White Acked-by: Victor Toso I don't have commit access there, do you? If not, you can submit a merge request and ask people to merge. Thanks for the review, and no, it looks like I don't. I've submitted a merge request, at least as far as I can tell... Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [x11spice 4/4] Add a --enable-dummy option to configure.
Provide some basic documentation as to how to use the new spice-video-dummy driver. Signed-off-by: Jeremy White --- Makefile.am | 3 +++ README | 19 +++ configure.ac| 7 +++ spice-video-dummy/spicedummy.sh | 30 ++ 4 files changed, 59 insertions(+) create mode 100755 spice-video-dummy/spicedummy.sh diff --git a/Makefile.am b/Makefile.am index b9452f6..cd23d81 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,7 @@ SUBDIRS = src +if DUMMY +SUBDIRS += spice-video-dummy +endif rpm: dist rpmbuild -ba --define "_sourcedir $$PWD" src/data/x11spice.spec diff --git a/README b/README index 34e8c1f..3dbee80 100644 --- a/README +++ b/README @@ -75,6 +75,25 @@ password you can use to connect. Refer to the x11spice man page for more details. +Spice Video Dummy Driver + +If you wish to create a 'headless' connection to a server - create an +X session without using the main system display - there is a straight +forward process that is now included with x11spice. + +That is, x11spice now optionally builds an Xorg driver that is based +on the dummy driver and the modesetting driver. This gives a virtual +frame buffer driver which should include modern Mesa capabilities. + +To use this driver, simply add: + + --enable-dummy + +to the configure line. This will build the Xorg driver. A simplistic +example is included showing the general concepts of using this driver. +Refer to the spice-video-dummy/spicedummy.sh bash script for more +information. + Using code coverage (gcov) - diff --git a/configure.ac b/configure.ac index cee7ec3..0acb4d2 100644 --- a/configure.ac +++ b/configure.ac @@ -17,6 +17,13 @@ PKG_CHECK_MODULES(PIXMAN, pixman-1) AM_CONDITIONAL([HAVE_GTEST], [pkg-config --atleast-version=2.38 glib-2.0]) +AC_ARG_ENABLE([dummy], AS_HELP_STRING([--enable-dummy], [Builds the spice-video-dummy driver]), [dummy=true]) +AM_CONDITIONAL([DUMMY], test x$dummy = xtrue) + +AS_IF([test "x$dummy" = "xtrue"], [ + AC_CONFIG_SUBDIRS([spice-video-dummy]) +]) + AC_PROG_SED # To use gcov, make sure you have the appropriate autoconf macors, # and uncomment this line, and the matching one in src/Makefile.am. diff --git a/spice-video-dummy/spicedummy.sh b/spice-video-dummy/spicedummy.sh new file mode 100755 index 000..c83934c --- /dev/null +++ b/spice-video-dummy/spicedummy.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Example script that shows how to launch the spice video driver +# and attach x11spice to it. This is not meant for production +# purposes. + +export DISPLAY=:2 + +password=`hexdump -n2 -e '2/1 "%02x"' /dev/urandom` +zerodir=`dirname $0` +dummydir=`(cd $zerodir; pwd)` +xmodules=`Xorg -showDefaultModulePath 2>&1` + +# Start Xorg +xinit -- `which Xorg` $DISPLAY \ +-modulepath "$dummydir/src/.libs,$xmodules" \ +-config "$dummydir/spicedummy.conf" & +xpid=$! +sleep 1 + +$dummydir/../src/x11spice --password=$password --allow-control & +spicepid=$! + +echo Xorg server started as pid $xpid +echo Spice server started as pid $spicepid +echo You should be able to connect with a spice client now to port 5900, +echo using a password of $password + +wait $spicepid + +kill $xpid -- 2.11.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [x11spice 2/4] Remove spaces and tabs at line ends.
Signed-off-by: Jeremy White
---
spice-video-dummy/src/dummy.h| 2 +-
spice-video-dummy/src/dummy_cursor.c | 8 ++---
spice-video-dummy/src/dummy_driver.c | 58 ++--
3 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..09cd917 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -36,7 +36,7 @@ typedef struct _color
int blue;
} dummy_colors;
-typedef struct dummyRec
+typedef struct dummyRec
{
/* options */
OptionInfoPtr Options;
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..9241d24 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -17,7 +17,7 @@ dummyShowCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-dPtr->DummyHWCursorShown = TRUE;
+dPtr->DummyHWCursorShown = TRUE;
}
static void
@@ -26,7 +26,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
- * turn cursor off
+ * turn cursor off
*
*/
dPtr->DummyHWCursorShown = FALSE;
@@ -47,7 +47,7 @@ static void
dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
dPtr->cursorFG = fg;
dPtr->cursorBG = bg;
}
@@ -94,7 +94,7 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
+
return(xf86InitCursor(pScreen, infoPtr));
}
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..9e29fe7 100644
--- a/spice-video-dummy/src/dummy_driver.c
+++ b/spice-video-dummy/src/dummy_driver.c
@@ -232,7 +232,7 @@ DUMMYProbe(DriverPtr drv, int flags)
for (i = 0; i < numUsed; i++) {
ScrnInfoPtr pScrn = NULL;
- int entityIndex =
+ int entityIndex =
xf86ClaimNoSlot(drv,DUMMY_CHIP,devSections[i],TRUE);
/* Allocate a ScrnInfoRec and claim the slot */
if ((pScrn = xf86AllocateScreen(drv,0 ))) {
@@ -253,7 +253,7 @@ DUMMYProbe(DriverPtr drv, int flags)
foundScreen = TRUE;
}
}
-}
+}
free(devSections);
@@ -275,21 +275,21 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
int maxClock = 30;
GDevPtr device = xf86GetEntityInfo(pScrn->entityList[0])->device;
-if (flags & PROBE_DETECT)
+if (flags & PROBE_DETECT)
return TRUE;
-
+
/* Allocate the DummyRec driverPrivate */
if (!DUMMYGetRec(pScrn)) {
return FALSE;
}
-
+
dPtr = DUMMYPTR(pScrn);
pScrn->chipset = (char *)xf86TokenToString(DUMMYChipsets,
DUMMY_CHIP);
xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Chipset is a DUMMY\n");
-
+
pScrn->monitor = pScrn->confScreen->monitor;
if (!xf86SetDepthBpp(pScrn, 0, 0, 0, Support24bppFb | Support32bppFb))
@@ -335,7 +335,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
}
}
-if (!xf86SetDefaultVisual(pScrn, -1))
+if (!xf86SetDefaultVisual(pScrn, -1))
return FALSE;
if (pScrn->depth > 1) {
@@ -364,7 +364,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
xf86DrvMsg(pScrn->scrnIndex, X_PROBED, "VideoRAM: %d kByte\n",
pScrn->videoRam);
}
-
+
if (device->dacSpeeds[0] != 0) {
maxClock = device->dacSpeeds[0];
xf86DrvMsg(pScrn->scrnIndex, X_CONFIG, "Max Clock: %d kHz\n",
@@ -385,7 +385,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
clockRanges->minClock = 11000; /* guessed §§§ */
clockRanges->maxClock = maxClock;
clockRanges->clockIndex = -1; /* programmable */
-clockRanges->interlaceAllowed = TRUE;
+clockRanges->interlaceAllowed = TRUE;
clockRanges->doubleScanAllowed = TRUE;
/* Subtract memory for HW cursor */
@@ -421,8 +421,8 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
* driver and if the driver doesn't provide code to set them. They
* are not pre-initialised at all.
*/
-xf86SetCrtcForModes(pScrn, 0);
-
+xf86SetCrtcForModes(pScrn, 0);
+
/* Set the current mode to the first in the list */
pScrn->currentMode = pScrn->modes;
@@ -440,7 +440,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
if (!xf86LoadSubModule(pScrn, "ramdac"))
RETURN;
}
-
+
/* We have no contiguous physical fb in physical memory */
pScrn->memPhysBase = 0;
pScrn->fbOffset = 0;
@@ -474,11 +474,11 @@ DUMMYLoadPalette(
DUMMYPtr dPtr = DUMMYPTR(pScrn);
[Spice-devel] spice-video-dummy driver for x11spice
This patch series introduces a new xorg video driver into the x11spice tree. The motive is that we are looking to have headless Xspice sessions with modern DRI / OpenGL support, and the current xf86-video-qxl driver does not lend itself well to that. The thinking is that if we combine x11spice with a modern dummy driver, we should be able to leverage all of the recent work in Xorg and make the entire stack simpler as well. This driver is based on the dummy driver, and then extended using pieces from the modesetting driver. The work was largely done by Henri Verbeet. I've pulled it into the x11spice tree and prepared it for submission. My plan is to include this in x11spice for now and develop the concept. Long term, it may make sense to deprecate the Xspice usage in the xf86-video-qxl driver and move that here, and perhaps move the spice-dummy-driver into the xorg tree. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [x11spice 1/4] Add the upstream xf86-video-dummy as of 850c0516.
This gives x11spice access to a modern virtual/offscreen frame buffer driver for the Xorg X server. We will then make slight revisions to make it more suitable for use with Spice. The overall result should be a more functional stack, as compared to the existing xf86-video-qxl driver, as we gain a wide range of Mesa features for free. It also simplifies the overall process. Signed-off-by: Jeremy White --- spice-video-dummy/COPYING| 2 + spice-video-dummy/Makefile.am| 31 ++ spice-video-dummy/README.md | 18 + spice-video-dummy/autogen.sh | 17 + spice-video-dummy/configure.ac | 72 spice-video-dummy/src/Makefile.am| 39 ++ spice-video-dummy/src/compat-api.h | 101 + spice-video-dummy/src/dummy.h| 59 +++ spice-video-dummy/src/dummy_cursor.c | 102 + spice-video-dummy/src/dummy_driver.c | 740 +++ 10 files changed, 1181 insertions(+) create mode 100644 spice-video-dummy/COPYING create mode 100644 spice-video-dummy/Makefile.am create mode 100644 spice-video-dummy/README.md create mode 100755 spice-video-dummy/autogen.sh create mode 100644 spice-video-dummy/configure.ac create mode 100644 spice-video-dummy/src/Makefile.am create mode 100644 spice-video-dummy/src/compat-api.h create mode 100644 spice-video-dummy/src/dummy.h create mode 100644 spice-video-dummy/src/dummy_cursor.c create mode 100644 spice-video-dummy/src/dummy_driver.c diff --git a/spice-video-dummy/COPYING b/spice-video-dummy/COPYING new file mode 100644 index 000..3c1ef48 --- /dev/null +++ b/spice-video-dummy/COPYING @@ -0,0 +1,2 @@ +Copyright 2002, SuSE Linux AG, Author: Egbert Eich + diff --git a/spice-video-dummy/Makefile.am b/spice-video-dummy/Makefile.am new file mode 100644 index 000..1a10a1c --- /dev/null +++ b/spice-video-dummy/Makefile.am @@ -0,0 +1,31 @@ +# Copyright 2005 Adam Jackson. +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# on the rights to use, copy, modify, merge, publish, distribute, sub +# license, and/or sell copies of the Software, and to permit persons to whom +# the Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL +# ADAM JACKSON BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +# IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +SUBDIRS = src +MAINTAINERCLEANFILES = ChangeLog + +.PHONY: ChangeLog + +ChangeLog: + $(CHANGELOG_CMD) + +dist-hook: ChangeLog + +EXTRA_DIST = README.md diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md new file mode 100644 index 000..2a84bc3 --- /dev/null +++ b/spice-video-dummy/README.md @@ -0,0 +1,18 @@ +xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server +-- + +All questions regarding this software should be directed at the +Xorg mailing list: + + https://lists.x.org/mailman/listinfo/xorg + +The master development code repository can be found at: + + https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy + +Please submit bug reports and requests to merge patches there. + +For patch submission instructions, see: + + https://www.x.org/wiki/Development/Documentation/SubmittingPatches + diff --git a/spice-video-dummy/autogen.sh b/spice-video-dummy/autogen.sh new file mode 100755 index 000..9b9f7e7 --- /dev/null +++ b/spice-video-dummy/autogen.sh @@ -0,0 +1,17 @@ +#! /bin/sh + +srcdir=`dirname "$0"` +test -z "$srcdir" && srcdir=. + +ORIGDIR=`pwd` +cd "$srcdir" + +autoreconf -v --install || exit 1 +cd "$ORIGDIR" || exit $? + +git config --local --get format.subjectPrefix >/dev/null 2>&1 || +git config --local format.subjectPrefix "PATCH xf86-video-dummy" + +if test -z "$NOCONFIGURE"; then +exec "$srcdir"/configure "$@" +fi diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac new file mode 100644 index 000..2ca0327 --- /dev/null +++ b/spice-video-dummy/configure.ac @@ -0,0 +1,72 @@ +# Copyright 2005 Adam Jackson. +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated document
[Spice-devel] [x11spice 3/4] Initial set of revisions to the dummy driver to add DRI and Present.
This makes the dummy driver similar to the mode setting driver.
This work was done by Henri Verbeet, which was in turn based heavily on
the Xorg modesetting driver.
Signed-off-by: Jeremy White
---
spice-video-dummy/.gitignore | 9 +
spice-video-dummy/README.md| 15 +-
spice-video-dummy/configure.ac | 12 +-
spice-video-dummy/spicedummy.conf | 18 ++
spice-video-dummy/src/Makefile.am | 16 +-
spice-video-dummy/src/dri2.c | 291 +
spice-video-dummy/src/dummy.h | 11 +-
spice-video-dummy/src/present.c| 104
.../src/{dummy_driver.c => spicedummy_driver.c}| 88 ++-
9 files changed, 530 insertions(+), 34 deletions(-)
create mode 100644 spice-video-dummy/.gitignore
create mode 100644 spice-video-dummy/spicedummy.conf
create mode 100644 spice-video-dummy/src/dri2.c
create mode 100644 spice-video-dummy/src/present.c
rename spice-video-dummy/src/{dummy_driver.c => spicedummy_driver.c} (86%)
diff --git a/spice-video-dummy/.gitignore b/spice-video-dummy/.gitignore
new file mode 100644
index 000..9b4b11f
--- /dev/null
+++ b/spice-video-dummy/.gitignore
@@ -0,0 +1,9 @@
+config.h
+config.h.in
+libtool
+ltmain.sh
+stamp-h1
+src/.libs
+*.o
+*.lo
+*.la
diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md
index 2a84bc3..3e77e4d 100644
--- a/spice-video-dummy/README.md
+++ b/spice-video-dummy/README.md
@@ -1,18 +1,15 @@
-xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
+spice-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
--
All questions regarding this software should be directed at the
-Xorg mailing list:
+spice-devel mailing list:
- https://lists.x.org/mailman/listinfo/xorg
+ https://lists.freedesktop.org/mailman/listinfo/spice-devel
-The master development code repository can be found at:
+This driver originated with the dummy driver from:
https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy
-Please submit bug reports and requests to merge patches there.
-
-For patch submission instructions, see:
-
- https://www.x.org/wiki/Development/Documentation/SubmittingPatches
+and modified to be similar to the mode setting driver:
+
https://gitlab.freedesktop.org/xorg/xserver/tree/master/hw/xfree86/drivers/modesetting
diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac
index 2ca0327..e85b7e5 100644
--- a/spice-video-dummy/configure.ac
+++ b/spice-video-dummy/configure.ac
@@ -22,10 +22,10 @@
# Initialize Autoconf
AC_PREREQ([2.60])
-AC_INIT([xf86-video-dummy],
-[0.3.8],
-[https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy/issues],
-[xf86-video-dummy])
+AC_INIT([spice-video-dummy],
+[1.1],
+[https://gitlab.freedesktop.org/spice/x11spice/issues],
+[spice-video-dummy])
AC_CONFIG_SRCDIR([Makefile.am])
AC_CONFIG_HEADERS([config.h])
AC_CONFIG_AUX_DIR(.)
@@ -61,10 +61,6 @@ PKG_CHECK_MODULES(XORG, [xorg-server >= 1.4.99.901] xproto
fontsproto $REQUIRED_
# Checks for libraries.
-
-DRIVER_NAME=dummy
-AC_SUBST([DRIVER_NAME])
-
AC_CONFIG_FILES([
Makefile
src/Makefile
diff --git a/spice-video-dummy/spicedummy.conf
b/spice-video-dummy/spicedummy.conf
new file mode 100644
index 000..950dd6f
--- /dev/null
+++ b/spice-video-dummy/spicedummy.conf
@@ -0,0 +1,18 @@
+Section "Device"
+Identifier "SpiceDummy"
+Driver "spicedummy"
+# The SWcursor option enables a software cursor. Default false.
+#Option "SWcursor" "false"
+# AccelMethod can be glamor (the default), or none.
+#Option "AccelMethod" "glamor"
+# kmsdev specifies which framebuffer device to use, default
/dev/dri/renderD128
+#Option "kmsdev" "/dev/dri/renderD128"
+VideoRam 16384
+EndSection
+
+Section "Screen"
+IDentifier "SpiceDummy"
+SubSection "Display"
+Virtual 1920 1200
+EndSubSection
+EndSection
diff --git a/spice-video-dummy/src/Makefile.am
b/spice-video-dummy/src/Makefile.am
index c0d82e0..6befa46 100644
--- a/spice-video-dummy/src/Makefile.am
+++ b/spice-video-dummy/src/Makefile.am
@@ -27,13 +27,15 @@
AM_CFLAGS = $(XORG_CFLAGS) $(PCIACCESS_CFLAGS)
-dummy_drv_la_LTLIBRARIES = dummy_drv.la
-dummy_drv_la_LDFLAGS = -module -avoid-version
-dummy_drv_la_LIBADD = $(XORG_LIBS)
-dummy_drv_ladir = @moduledir@/drivers
+spicedummy_drv_la_LTLIBRARIES = spicedummy_drv.la
+spicedummy_drv_la_LDFLAGS = -module -avoid-version
+spicedummy_drv_la_LIBADD = $(XORG_LIBS)
+spicedummy_drv_ladir = @moduledir@/drivers
-dummy_drv_la_SOURCES = \
+spicedummy_drv_la_SOURCES = \
compat
Re: [Spice-devel] [x11spice 2/4] Remove spaces and tabs at line ends.
On 4/18/19 3:10 AM, Frediano Ziglio wrote:
Signed-off-by: Jeremy White
Why this commit is not merged to 1/4 ?
My rationale was that this approach left 1/4 as binary identical to the
upstream repo, leaving what seemed to me like a more clear historical
record.
I don't feel strongly about it, if you feel that condensing them would
be better, I'm happy to spin them that way.
Cheers,
Jeremy
---
spice-video-dummy/src/dummy.h| 2 +-
spice-video-dummy/src/dummy_cursor.c | 8 ++---
spice-video-dummy/src/dummy_driver.c | 58
++--
3 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..09cd917 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -36,7 +36,7 @@ typedef struct _color
int blue;
} dummy_colors;
-typedef struct dummyRec
+typedef struct dummyRec
{
/* options */
OptionInfoPtr Options;
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..9241d24 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -17,7 +17,7 @@ dummyShowCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-dPtr->DummyHWCursorShown = TRUE;
+dPtr->DummyHWCursorShown = TRUE;
}
static void
@@ -26,7 +26,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
- * turn cursor off
+ * turn cursor off
*
*/
dPtr->DummyHWCursorShown = FALSE;
@@ -47,7 +47,7 @@ static void
dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
dPtr->cursorFG = fg;
dPtr->cursorBG = bg;
}
@@ -94,7 +94,7 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
+
return(xf86InitCursor(pScreen, infoPtr));
}
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..9e29fe7 100644
--- a/spice-video-dummy/src/dummy_driver.c
+++ b/spice-video-dummy/src/dummy_driver.c
@@ -232,7 +232,7 @@ DUMMYProbe(DriverPtr drv, int flags)
for (i = 0; i < numUsed; i++) {
ScrnInfoPtr pScrn = NULL;
- int entityIndex =
+ int entityIndex =
xf86ClaimNoSlot(drv,DUMMY_CHIP,devSections[i],TRUE);
/* Allocate a ScrnInfoRec and claim the slot */
if ((pScrn = xf86AllocateScreen(drv,0 ))) {
@@ -253,7 +253,7 @@ DUMMYProbe(DriverPtr drv, int flags)
foundScreen = TRUE;
}
}
-}
+}
free(devSections);
@@ -275,21 +275,21 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
int maxClock = 30;
GDevPtr device = xf86GetEntityInfo(pScrn->entityList[0])->device;
-if (flags & PROBE_DETECT)
+if (flags & PROBE_DETECT)
return TRUE;
-
+
Here there are no brackets
/* Allocate the DummyRec driverPrivate */
if (!DUMMYGetRec(pScrn)) {
return FALSE;
}
Here there are.
Also the spacing is not consistent. Spaces and tabs are mixed.
Which coding style are these files?
-
+
dPtr = DUMMYPTR(pScrn);
pScrn->chipset = (char *)xf86TokenToString(DUMMYChipsets,
DUMMY_CHIP);
xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Chipset is a DUMMY\n");
-
+
pScrn->monitor = pScrn->confScreen->monitor;
if (!xf86SetDepthBpp(pScrn, 0, 0, 0, Support24bppFb | Support32bppFb))
@@ -335,7 +335,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
}
}
-if (!xf86SetDefaultVisual(pScrn, -1))
+if (!xf86SetDefaultVisual(pScrn, -1))
return FALSE;
if (pScrn->depth > 1) {
@@ -364,7 +364,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
xf86DrvMsg(pScrn->scrnIndex, X_PROBED, "VideoRAM: %d kByte\n",
pScrn->videoRam);
}
-
+
if (device->dacSpeeds[0] != 0) {
maxClock = device->dacSpeeds[0];
xf86DrvMsg(pScrn->scrnIndex, X_CONFIG, "Max Clock: %d kHz\n",
@@ -385,7 +385,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
clockRanges->minClock = 11000; /* guessed §§§ */
clockRanges->maxClock = maxClock;
clockRanges->clockIndex = -1; /* programmable */
-clockRanges->interlaceAllowed = TRUE;
+clockRanges->interlaceAllowed = TRUE;
clockRanges->doubleScanAllowed = TRUE;
/* Subtract memory for HW cursor */
@@ -421,8 +421,8 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
* driver and if the driver doesn't provide code to set them. They
* ar
Re: [Spice-devel] [x11spice 2/4] Remove spaces and tabs at line ends.
Maybe in 1/4 specify from which repository (with url) 850c0516 came?
Sure.
However I would like to have a consistent style. As I can see you
are planning to do a lot of changes on this "dummy" driver so I would
prefer to change all style to a more SPICE one. I can see at least 4
different styles for just the "if".
Well, I think the long term plan is to persuade the Xorg guys to accept
this driver in one form or another. (Perhaps as a modification to the
dummy driver itself). My thought was to have it start in x11spice,
demonstrate a viable purpose for it, and then propose to move it to the
xorg tree in some form.
Henri tends to follow the Wine rules on style - don't change what it is
already there. It's not a lot of code, so it would be easy to make it
follow SPICE style guidelines for now.
Fine with me to keep the 1/4 as an exact copy of the original driver.
Have you also not considered coming out with a proper name instead of
"spicedummy"? I don't know... x11spice for instance could suit.
I don't know that I feel strongly about naming, but x11spice would be a
mistake, IMHO. x11spice has a specific meaning right now, and I'd
rather not muddy that.
The rationale for the present name is that 'xf86-video-dummy' is the
Xorg name for the replacement for Xvfb. So in Xorg parlance, a headless
server uses the 'dummy' driver. So sharing that name, in theory, makes
the purpose of the driver clear.
Cheers,
Jeremy
I don't feel strongly about it, if you feel that condensing them would
be better, I'm happy to spin them that way.
Cheers,
Jeremy
---
spice-video-dummy/src/dummy.h| 2 +-
spice-video-dummy/src/dummy_cursor.c | 8 ++---
spice-video-dummy/src/dummy_driver.c | 58
++--
3 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..09cd917 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -36,7 +36,7 @@ typedef struct _color
int blue;
} dummy_colors;
-typedef struct dummyRec
+typedef struct dummyRec
{
/* options */
OptionInfoPtr Options;
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..9241d24 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -17,7 +17,7 @@ dummyShowCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-dPtr->DummyHWCursorShown = TRUE;
+dPtr->DummyHWCursorShown = TRUE;
}
static void
@@ -26,7 +26,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
- * turn cursor off
+ * turn cursor off
*
*/
dPtr->DummyHWCursorShown = FALSE;
@@ -47,7 +47,7 @@ static void
dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
dPtr->cursorFG = fg;
dPtr->cursorBG = bg;
}
@@ -94,7 +94,7 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
+
return(xf86InitCursor(pScreen, infoPtr));
}
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..9e29fe7 100644
--- a/spice-video-dummy/src/dummy_driver.c
+++ b/spice-video-dummy/src/dummy_driver.c
@@ -232,7 +232,7 @@ DUMMYProbe(DriverPtr drv, int flags)
for (i = 0; i < numUsed; i++) {
ScrnInfoPtr pScrn = NULL;
- int entityIndex =
+ int entityIndex =
xf86ClaimNoSlot(drv,DUMMY_CHIP,devSections[i],TRUE);
/* Allocate a ScrnInfoRec and claim the slot */
if ((pScrn = xf86AllocateScreen(drv,0 ))) {
@@ -253,7 +253,7 @@ DUMMYProbe(DriverPtr drv, int flags)
foundScreen = TRUE;
}
}
-}
+}
free(devSections);
@@ -275,21 +275,21 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
int maxClock = 30;
GDevPtr device = xf86GetEntityInfo(pScrn->entityList[0])->device;
-if (flags & PROBE_DETECT)
+if (flags & PROBE_DETECT)
return TRUE;
-
+
Here there are no brackets
/* Allocate the DummyRec driverPrivate */
if (!DUMMYGetRec(pScrn)) {
return FALSE;
}
Here there are.
Also the spacing is not consistent. Spaces and tabs are mixed.
Which coding style are these files?
-
+
dPtr = DUMMYPTR(pScrn);
pScrn->chipset = (char *)xf86TokenToString(DUMMYChipsets,
DUMMY_CHIP);
xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Chipset is a DUMMY\n");
-
+
pScrn->monitor = pScrn->confScreen->monitor;
if (!xf86SetDepthBpp(pScrn, 0, 0, 0, Support24bppFb |
Support32bppFb)
Re: [Spice-devel] gtk client side timeouts
Hi, On 3/10/17 11:01 AM, Christophe Fergeau wrote: On Fri, Mar 10, 2017 at 10:49:54AM -0600, Jeremy White wrote: Hey all, We've got an issue with a remote viewer kiosk. That is, in our normal mode of operation, if something goes wrong with a client connection (e.g. the sessions is killed, server restarted normally), the kiosk will exit on disconnect, and we get a chance to retry the connection, or present the user with a 'server down' style message. But in the case of a serious network problem or a server hard power cycle (i.e. no TCP FIN packets can flow), our end user behavior is not ideal - the kiosk appears to hang solid, requiring a power cycle. That's because we've got the stock keepalive timeouts, or about 2 hours and 11 minutes, before the client sees the disconnect. Now this is a relatively rare occurrence, and arguably a minor nuisance, but I've been asked to see what it would take to improve the situation. I looked for past discussions on spice-devel; I only readily found discussion of the keepalive parameters on the server side, and it looks like we take the idle timer down to 10 minutes now, from 2 hours. I didn't find discussion of something similar client side. Have we considered tuning them on the spice-gtk client side? The attached patch applies a roughly 75 second timeout for me; is this something that would be viable to add to the gtk client, even if only as an option? Ah, I've coincidentally been looking at this this week too, not with the same goal in mind though, my problem is SPICE connection sometimes getting dropped if idle for too long. But the current answer is also tweaking TCP keepalives. Definitely no objection having some form of keepalive client sides, though your proposed change is not going to work for the Windows client. I've made some good progress on patches to add both Windows/linux support to glib, but need to finish them up ;) Ping? This problem persists, and the patch I sent then: https://lists.freedesktop.org/archives/spice-devel/attachments/20170310/6938c6dc/attachment.diff has been working for us for a number of years without any unexpected consequences. Could I submit this patch and we remove it when there is a better solution that also supports Windows? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] gtk client side timeouts
Ping? This problem persists, and the patch I sent then: https://lists.freedesktop.org/archives/spice-devel/attachments/20170310/6938c6dc/attachment.diff has been working for us for a number of years without any unexpected consequences. Could I submit this patch and we remove it when there is a better solution that also supports Windows? Cheers, Jeremy I think it was just a mistake. We have similar code in SPICE server and usbredir, just probably got forgotten. It needs only some portability enhancement (first would be Unix, then Windows). Can you send a proper patch with commit message? Actually, it looks like I said I would try to work up something for Windows, so if anyone dropped the ball, looks like it was me. I'm pretty far removed from Windows development at the moment, so I'll send a Linux only version. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk] Detect timeout conditions more aggressively on Linux
This mitigates a fairly rare problem we see with our kiosk mode clients.
That is, normally if something goes wrong with a client connection
(e.g. the session is killed, or the server is restarted ), the kiosk will
exit on disconnect, and we get a chance to retry the connection, or
present the user with a 'server down' style message.
But in the case of a serious network problem or a server hard power
cycle (i.e. no TCP FIN packets can flow), our end user behavior is not
ideal - the kiosk appears to hang solid, requiring a power cycle.
That's because we've got the stock keepalive timeouts, or about 2 hours
and 11 minutes, before the client sees the disconnect.
This change will cause the client to recognize the server has vanished
without a TCP FIN after 75 seconds.
See this thread:
https://lists.freedesktop.org/archives/spice-devel/2017-March/036553.html
As well as this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1436589
Signed-off-by: Jeremy White
---
configure.ac| 18 ++
meson.build | 15 +++
src/spice-session.c | 15 +++
3 files changed, 48 insertions(+)
diff --git a/configure.ac b/configure.ac
index 0666e2a8..9c64a8dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,24 @@ AM_CONDITIONAL([HAVE_EGL],[test "$have_egl" = "yes"])
AS_IF([test "$have_egl" = "yes"],
AC_DEFINE([HAVE_EGL], [1], [Define if supporting EGL]))
+AC_CHECK_DECL([TCP_KEEPIDLE], [have_tcp_keepidle="yes"],,
+ [#include ])
+AS_IF([test "x$have_tcp_keepidle" = "xyes"],
+ [AC_DEFINE([HAVE_TCP_KEEPIDLE],1,[Define to 1 if has a
TCP_KEEPIDLE definition])],
+)
+AC_CHECK_DECL([TCP_KEEPINTVL], [have_tcp_keepintvl="yes"],,
+ [#include ])
+AS_IF([test "x$have_tcp_keepintvl" = "xyes"],
+ [AC_DEFINE([HAVE_TCP_KEEPINTVL],1,[Define to 1 if has a
TCP_KEEPINTVL definition])],
+)
+AC_CHECK_DECL([TCP_KEEPCNT], [have_tcp_keepcnt="yes"],,
+ [#include ])
+AS_IF([test "x$have_tcp_keepcnt" = "xyes"],
+ [AC_DEFINE([HAVE_TCP_KEEPCNT],1,[Define to 1 if has a
TCP_KEEPCNT definition])],
+)
+
+
+
AC_CHECK_LIBM
AC_SUBST(LIBM)
diff --git a/meson.build b/meson.build
index e0fba930..f2ef94a9 100644
--- a/meson.build
+++ b/meson.build
@@ -72,6 +72,21 @@ foreach func : ['clearenv', 'strtok_r']
endif
endforeach
+# TCP_KEEPIDLE definition in netinet/tcp.h
+if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPIDLE')
+ spice_gtk_config_data.set('HAVE_TCP_KEEPIDLE', '1')
+endif
+
+# TCP_KEEPINTVL definition in netinet/tcp.h
+if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPINTVL')
+ spice_gtk_config_data.set('HAVE_TCP_KEEPINTVL', '1')
+endif
+
+# TCP_KEEPCNT definition in netinet/tcp.h
+if compiler.has_header_symbol('netinet/tcp.h', 'TCP_KEEPCNT')
+ spice_gtk_config_data.set('HAVE_TCP_KEEPCNT', '1')
+endif
+
#
# check for mandatory dependencies
#
diff --git a/src/spice-session.c b/src/spice-session.c
index 89528970..f6ec6a83 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -18,6 +18,7 @@
#include "config.h"
#include
+#include
#include
#ifdef G_OS_UNIX
#include
@@ -2254,6 +2255,20 @@ GSocketConnection*
spice_session_channel_open_host(SpiceSession *session, SpiceC
g_socket_set_timeout(socket, 0);
g_socket_set_blocking(socket, FALSE);
g_socket_set_keepalive(socket, TRUE);
+
+/* Make Linux client timeouts a bit more responsive */
+/* TODO: Support Windows. It appears as though you can
+ set KEEPIDLE and KEEPINTVL, but not KEEPCNT.
+ See SIO_KEEPALIVE_VALS */
+#ifdef HAVE_TCP_KEEPIDLE
+g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPIDLE, 30, NULL);
+#endif
+#ifdef HAVE_TCP_KEEPINTVL
+g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPINTVL, 15, NULL);
+#endif
+#ifdef HAVE_TCP_KEEPCNT
+g_socket_set_option(socket, IPPROTO_TCP, TCP_KEEPCNT, 3, NULL);
+#endif
}
g_clear_object(&open_host.client);
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk] Detect timeout conditions more aggressively on Linux
Can I suggest this follow up? https://gitlab.freedesktop.org/fziglio/spice-gtk/commit/e76f04625c075776f0ca0e78e5a42e4aa396faa7 Sure, that approach works fine. I'd probably recommend that the Windows interval be set to something lower than 15 seconds (from my reading, it looks like the count is fixed at 10, so that would be 180 seconds, which isn't a huge improvement). I'm not sure the best path forward; happy to ack your patch if that's the easiest way forward. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 4/4] Add a --enable-dummy option to configure.
Provide some basic documentation as to how to use the new spice-video-dummy driver. Signed-off-by: Jeremy White --- Makefile.am | 3 +++ README | 19 +++ configure.ac| 7 +++ spice-video-dummy/spicedummy.sh | 30 ++ 4 files changed, 59 insertions(+) create mode 100755 spice-video-dummy/spicedummy.sh diff --git a/Makefile.am b/Makefile.am index b9452f6..cd23d81 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,7 @@ SUBDIRS = src +if DUMMY +SUBDIRS += spice-video-dummy +endif rpm: dist rpmbuild -ba --define "_sourcedir $$PWD" src/data/x11spice.spec diff --git a/README b/README index 34e8c1f..3dbee80 100644 --- a/README +++ b/README @@ -75,6 +75,25 @@ password you can use to connect. Refer to the x11spice man page for more details. +Spice Video Dummy Driver + +If you wish to create a 'headless' connection to a server - create an +X session without using the main system display - there is a straight +forward process that is now included with x11spice. + +That is, x11spice now optionally builds an Xorg driver that is based +on the dummy driver and the modesetting driver. This gives a virtual +frame buffer driver which should include modern Mesa capabilities. + +To use this driver, simply add: + + --enable-dummy + +to the configure line. This will build the Xorg driver. A simplistic +example is included showing the general concepts of using this driver. +Refer to the spice-video-dummy/spicedummy.sh bash script for more +information. + Using code coverage (gcov) - diff --git a/configure.ac b/configure.ac index cee7ec3..0acb4d2 100644 --- a/configure.ac +++ b/configure.ac @@ -17,6 +17,13 @@ PKG_CHECK_MODULES(PIXMAN, pixman-1) AM_CONDITIONAL([HAVE_GTEST], [pkg-config --atleast-version=2.38 glib-2.0]) +AC_ARG_ENABLE([dummy], AS_HELP_STRING([--enable-dummy], [Builds the spice-video-dummy driver]), [dummy=true]) +AM_CONDITIONAL([DUMMY], test x$dummy = xtrue) + +AS_IF([test "x$dummy" = "xtrue"], [ + AC_CONFIG_SUBDIRS([spice-video-dummy]) +]) + AC_PROG_SED # To use gcov, make sure you have the appropriate autoconf macors, # and uncomment this line, and the matching one in src/Makefile.am. diff --git a/spice-video-dummy/spicedummy.sh b/spice-video-dummy/spicedummy.sh new file mode 100755 index 000..c83934c --- /dev/null +++ b/spice-video-dummy/spicedummy.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Example script that shows how to launch the spice video driver +# and attach x11spice to it. This is not meant for production +# purposes. + +export DISPLAY=:2 + +password=`hexdump -n2 -e '2/1 "%02x"' /dev/urandom` +zerodir=`dirname $0` +dummydir=`(cd $zerodir; pwd)` +xmodules=`Xorg -showDefaultModulePath 2>&1` + +# Start Xorg +xinit -- `which Xorg` $DISPLAY \ +-modulepath "$dummydir/src/.libs,$xmodules" \ +-config "$dummydir/spicedummy.conf" & +xpid=$! +sleep 1 + +$dummydir/../src/x11spice --password=$password --allow-control & +spicepid=$! + +echo Xorg server started as pid $xpid +echo Spice server started as pid $spicepid +echo You should be able to connect with a spice client now to port 5900, +echo using a password of $password + +wait $spicepid + +kill $xpid -- 2.11.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 1/4] Add the upstream xf86-video-dummy as of November 25, 2018.
This gives x11spice access to a modern virtual/offscreen frame buffer driver for the Xorg X server. We will then make slight revisions to make it more suitable for use with Spice. The overall result should be a more functional stack, as compared to the existing xf86-video-qxl driver, as we gain a wide range of Mesa features for free. It also simplifies the overall process. This is from https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy as of 850c0516. Signed-off-by: Jeremy White --- v2: Provide a specific URL for reference to the upstream xorg driver. --- spice-video-dummy/COPYING| 2 + spice-video-dummy/Makefile.am| 31 ++ spice-video-dummy/README.md | 18 + spice-video-dummy/autogen.sh | 17 + spice-video-dummy/configure.ac | 72 spice-video-dummy/src/Makefile.am| 39 ++ spice-video-dummy/src/compat-api.h | 101 + spice-video-dummy/src/dummy.h| 59 +++ spice-video-dummy/src/dummy_cursor.c | 102 + spice-video-dummy/src/dummy_driver.c | 740 +++ 10 files changed, 1181 insertions(+) create mode 100644 spice-video-dummy/COPYING create mode 100644 spice-video-dummy/Makefile.am create mode 100644 spice-video-dummy/README.md create mode 100755 spice-video-dummy/autogen.sh create mode 100644 spice-video-dummy/configure.ac create mode 100644 spice-video-dummy/src/Makefile.am create mode 100644 spice-video-dummy/src/compat-api.h create mode 100644 spice-video-dummy/src/dummy.h create mode 100644 spice-video-dummy/src/dummy_cursor.c create mode 100644 spice-video-dummy/src/dummy_driver.c diff --git a/spice-video-dummy/COPYING b/spice-video-dummy/COPYING new file mode 100644 index 000..3c1ef48 --- /dev/null +++ b/spice-video-dummy/COPYING @@ -0,0 +1,2 @@ +Copyright 2002, SuSE Linux AG, Author: Egbert Eich + diff --git a/spice-video-dummy/Makefile.am b/spice-video-dummy/Makefile.am new file mode 100644 index 000..1a10a1c --- /dev/null +++ b/spice-video-dummy/Makefile.am @@ -0,0 +1,31 @@ +# Copyright 2005 Adam Jackson. +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# on the rights to use, copy, modify, merge, publish, distribute, sub +# license, and/or sell copies of the Software, and to permit persons to whom +# the Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL +# ADAM JACKSON BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +# IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +SUBDIRS = src +MAINTAINERCLEANFILES = ChangeLog + +.PHONY: ChangeLog + +ChangeLog: + $(CHANGELOG_CMD) + +dist-hook: ChangeLog + +EXTRA_DIST = README.md diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md new file mode 100644 index 000..2a84bc3 --- /dev/null +++ b/spice-video-dummy/README.md @@ -0,0 +1,18 @@ +xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server +-- + +All questions regarding this software should be directed at the +Xorg mailing list: + + https://lists.x.org/mailman/listinfo/xorg + +The master development code repository can be found at: + + https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy + +Please submit bug reports and requests to merge patches there. + +For patch submission instructions, see: + + https://www.x.org/wiki/Development/Documentation/SubmittingPatches + diff --git a/spice-video-dummy/autogen.sh b/spice-video-dummy/autogen.sh new file mode 100755 index 000..9b9f7e7 --- /dev/null +++ b/spice-video-dummy/autogen.sh @@ -0,0 +1,17 @@ +#! /bin/sh + +srcdir=`dirname "$0"` +test -z "$srcdir" && srcdir=. + +ORIGDIR=`pwd` +cd "$srcdir" + +autoreconf -v --install || exit 1 +cd "$ORIGDIR" || exit $? + +git config --local --get format.subjectPrefix >/dev/null 2>&1 || +git config --local format.subjectPrefix "PATCH xf86-video-dummy" + +if test -z "$NOCONFIGURE"; then +exec "$srcdir"/configure "$@" +fi diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac new file mode 100644 index 000..2ca0327 --- /dev/null +++ b/spice-video-dummy/configure.ac @@ -0,0 +1
[Spice-devel] [PATCH v2 x11spice 3/4] Initial set of revisions to the dummy driver to add DRI and Present.
This makes the dummy driver similar to the mode setting driver.
This work was done by Henri Verbeet, which was in turn based heavily on
the Xorg modesetting driver.
Signed-off-by: Jeremy White
---
spice-video-dummy/.gitignore | 9 +
spice-video-dummy/README.md| 15 +-
spice-video-dummy/configure.ac | 12 +-
spice-video-dummy/spicedummy.conf | 18 ++
spice-video-dummy/src/Makefile.am | 16 +-
spice-video-dummy/src/dri2.c | 288 +
spice-video-dummy/src/dummy_cursor.c | 2 +-
spice-video-dummy/src/present.c| 104
spice-video-dummy/src/{dummy.h => spicedummy.h}| 10 +
.../src/{dummy_driver.c => spicedummy_driver.c}| 82 +-
10 files changed, 523 insertions(+), 33 deletions(-)
create mode 100644 spice-video-dummy/.gitignore
create mode 100644 spice-video-dummy/spicedummy.conf
create mode 100644 spice-video-dummy/src/dri2.c
create mode 100644 spice-video-dummy/src/present.c
rename spice-video-dummy/src/{dummy.h => spicedummy.h} (83%)
rename spice-video-dummy/src/{dummy_driver.c => spicedummy_driver.c} (88%)
diff --git a/spice-video-dummy/.gitignore b/spice-video-dummy/.gitignore
new file mode 100644
index 000..9b4b11f
--- /dev/null
+++ b/spice-video-dummy/.gitignore
@@ -0,0 +1,9 @@
+config.h
+config.h.in
+libtool
+ltmain.sh
+stamp-h1
+src/.libs
+*.o
+*.lo
+*.la
diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md
index 2a84bc3..3e77e4d 100644
--- a/spice-video-dummy/README.md
+++ b/spice-video-dummy/README.md
@@ -1,18 +1,15 @@
-xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
+spice-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
--
All questions regarding this software should be directed at the
-Xorg mailing list:
+spice-devel mailing list:
- https://lists.x.org/mailman/listinfo/xorg
+ https://lists.freedesktop.org/mailman/listinfo/spice-devel
-The master development code repository can be found at:
+This driver originated with the dummy driver from:
https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy
-Please submit bug reports and requests to merge patches there.
-
-For patch submission instructions, see:
-
- https://www.x.org/wiki/Development/Documentation/SubmittingPatches
+and modified to be similar to the mode setting driver:
+
https://gitlab.freedesktop.org/xorg/xserver/tree/master/hw/xfree86/drivers/modesetting
diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac
index 2ca0327..e85b7e5 100644
--- a/spice-video-dummy/configure.ac
+++ b/spice-video-dummy/configure.ac
@@ -22,10 +22,10 @@
# Initialize Autoconf
AC_PREREQ([2.60])
-AC_INIT([xf86-video-dummy],
-[0.3.8],
-[https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy/issues],
-[xf86-video-dummy])
+AC_INIT([spice-video-dummy],
+[1.1],
+[https://gitlab.freedesktop.org/spice/x11spice/issues],
+[spice-video-dummy])
AC_CONFIG_SRCDIR([Makefile.am])
AC_CONFIG_HEADERS([config.h])
AC_CONFIG_AUX_DIR(.)
@@ -61,10 +61,6 @@ PKG_CHECK_MODULES(XORG, [xorg-server >= 1.4.99.901] xproto
fontsproto $REQUIRED_
# Checks for libraries.
-
-DRIVER_NAME=dummy
-AC_SUBST([DRIVER_NAME])
-
AC_CONFIG_FILES([
Makefile
src/Makefile
diff --git a/spice-video-dummy/spicedummy.conf
b/spice-video-dummy/spicedummy.conf
new file mode 100644
index 000..950dd6f
--- /dev/null
+++ b/spice-video-dummy/spicedummy.conf
@@ -0,0 +1,18 @@
+Section "Device"
+Identifier "SpiceDummy"
+Driver "spicedummy"
+# The SWcursor option enables a software cursor. Default false.
+#Option "SWcursor" "false"
+# AccelMethod can be glamor (the default), or none.
+#Option "AccelMethod" "glamor"
+# kmsdev specifies which framebuffer device to use, default
/dev/dri/renderD128
+#Option "kmsdev" "/dev/dri/renderD128"
+VideoRam 16384
+EndSection
+
+Section "Screen"
+IDentifier "SpiceDummy"
+SubSection "Display"
+Virtual 1920 1200
+EndSubSection
+EndSection
diff --git a/spice-video-dummy/src/Makefile.am
b/spice-video-dummy/src/Makefile.am
index c0d82e0..3c03409 100644
--- a/spice-video-dummy/src/Makefile.am
+++ b/spice-video-dummy/src/Makefile.am
@@ -27,13 +27,15 @@
AM_CFLAGS = $(XORG_CFLAGS) $(PCIACCESS_CFLAGS)
-dummy_drv_la_LTLIBRARIES = dummy_drv.la
-dummy_drv_la_LDFLAGS = -module -avoid-version
-dummy_drv_la_LIBADD = $(XORG_LIBS)
-dummy_drv_ladir = @moduledir@/drivers
+spicedummy_drv_la_LTLIBRARIES = spicedummy_drv.la
+spicedummy_drv_la_LDFLAGS = -module -avoid-version
+spicedummy_drv_la_LIBADD = $(X
[Spice-devel] [PATCH v2 x11spice 2/4] Apply Spice style to the spice-video-dummy
Signed-off-by: Jeremy White
---
spice-video-dummy/src/dummy.h| 9 +-
spice-video-dummy/src/dummy_cursor.c | 42 +--
spice-video-dummy/src/dummy_driver.c | 558 ---
3 files changed, 281 insertions(+), 328 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..0c16591 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -29,15 +29,13 @@ extern void DUMMYShowCursor(ScrnInfoPtr pScrn);
extern void DUMMYHideCursor(ScrnInfoPtr pScrn);
/* globals */
-typedef struct _color
-{
+typedef struct _color {
int red;
int green;
int blue;
} dummy_colors;
-typedef struct dummyRec
-{
+typedef struct dummyRec {
/* options */
OptionInfoPtr Options;
Bool swCursor;
@@ -50,10 +48,9 @@ typedef struct dummyRec
int cursorFG, cursorBG;
dummy_colors colors[1024];
-Bool(*CreateWindow)() ; /* wrapped CreateWindow */
+ Bool(*CreateWindow) ();/* wrapped CreateWindow */
Bool prop;
} DUMMYRec, *DUMMYPtr;
/* The privates of the DUMMY driver */
#define DUMMYPTR(p)((DUMMYPtr)((p)->driverPrivate))
-
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..ccce442 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -11,22 +11,20 @@
/* Driver specific headers */
#include "dummy.h"
-static void
-dummyShowCursor(ScrnInfoPtr pScrn)
+static void dummyShowCursor(ScrnInfoPtr pScrn)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-dPtr->DummyHWCursorShown = TRUE;
+dPtr->DummyHWCursorShown = TRUE;
}
-static void
-dummyHideCursor(ScrnInfoPtr pScrn)
+static void dummyHideCursor(ScrnInfoPtr pScrn)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
- * turn cursor off
+ * turn cursor off
*
*/
dPtr->DummyHWCursorShown = FALSE;
@@ -34,8 +32,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
#define MAX_CURS 64
-static void
-dummySetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
+static void dummySetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
@@ -43,43 +40,39 @@ dummySetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
dPtr->cursorY = y;
}
-static void
-dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
+static void dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
dPtr->cursorFG = fg;
dPtr->cursorBG = bg;
}
-static void
-dummyLoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
+static void dummyLoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
}
-static Bool
-dummyUseHWCursor(ScreenPtr pScr, CursorPtr pCurs)
+static Bool dummyUseHWCursor(ScreenPtr pScr, CursorPtr pCurs)
{
DUMMYPtr dPtr = DUMMYPTR(xf86ScreenToScrn(pScr));
-return(!dPtr->swCursor);
+return (!dPtr->swCursor);
}
#if 0
-static unsigned char*
-dummyRealizeCursor(xf86CursorInfoPtr infoPtr, CursorPtr pCurs)
+static unsigned char *dummyRealizeCursor(xf86CursorInfoPtr infoPtr, CursorPtr
pCurs)
{
return NULL;
}
#endif
-Bool
-DUMMYCursorInit(ScreenPtr pScreen)
+Bool DUMMYCursorInit(ScreenPtr pScreen)
{
DUMMYPtr dPtr = DUMMYPTR(xf86ScreenToScrn(pScreen));
xf86CursorInfoPtr infoPtr;
infoPtr = xf86CreateCursorInfoRec();
-if(!infoPtr) return FALSE;
+if (!infoPtr)
+return FALSE;
dPtr->CursorInfo = infoPtr;
@@ -94,9 +87,6 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
-return(xf86InitCursor(pScreen, infoPtr));
-}
-
-
+return (xf86InitCursor(pScreen, infoPtr));
+}
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..0696151 100644
--- a/spice-video-dummy/src/dummy_driver.c
+++ b/spice-video-dummy/src/dummy_driver.c
@@ -41,23 +41,21 @@
#include "servermd.h"
/* Mandatory functions */
-static const OptionInfoRec * DUMMYAvailableOptions(int chipid, int busid);
-static void DUMMYIdentify(int flags);
-static Bool DUMMYProbe(DriverPtr drv, int flags);
-static Bool DUMMYPreInit(ScrnInfoPtr pScrn, int flags);
-static Bool DUMMYScreenInit(SCREEN_INIT_ARGS_DECL);
-static Bool DUMMYEnterVT(VT_FUNC_ARGS_DECL);
-static void DUMMYLeaveVT(VT_FUNC_ARGS_DECL);
-static Bool DUMMYCloseScreen(CLOSE_SCREEN_ARGS_DECL);
-static Bool DUMMYCreateWindow(WindowPtr pWin);
-static void DUMMYFreeScreen(FREE_SCREEN_ARGS_DECL);
-static ModeStatus DUMMYValidMode(SCRN_ARG_TYPE arg, DisplayModePtr mode,
- Bool verbose, int flags);
-static BoolDUMMYSaveScreen(ScreenPtr pScreen, int mode);
+static const OptionInfoRec *DUMMYAvailableOptions(i
Re: [Spice-devel] [PATCH v2 x11spice 2/4] Apply Spice style to the spice-video-dummy
Hi Frediano,
Thanks for the review. I've condensed the thread to just a few
interesting points; for the most part, I was able to get indent to make
the changes you requested, and will resubmit with that.
+if (!infoPtr)
+return FALSE;
What we usually want is always brackets for if so
if (!infoPtr) {
return FALSE;
}
(this should be doable using indent, not sure how).
I did not find a way. I've chosen not to try to fix these, there are a
fair number from the upstream code, and I worry that manual edits will
create bugs. If this particular style convention is absolutely vital, I
can do that work as well, he said grudgingly .
dPtr->CursorInfo = infoPtr;
@@ -94,9 +87,6 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
-return(xf86InitCursor(pScreen, infoPtr));
-}
-
-
+return (xf86InitCursor(pScreen, infoPtr));
I would remove the parenthesis, like
return xf86InitCursor(pScreen, infoPtr);
(not strong)
(not changed) :-/.
Cheers,
Jeremy
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 x11spice 1/5] Add the upstream xf86-video-dummy as of November 25, 2018.
This gives x11spice access to a modern virtual/offscreen frame buffer driver for the Xorg X server. We will then make slight revisions to make it more suitable for use with Spice. The overall result should be a more functional stack, as compared to the existing xf86-video-qxl driver, as we gain a wide range of Mesa features for free. It also simplifies the overall process. This is from https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy as of 850c0516. Signed-off-by: Jeremy White --- v2: Provide a specific URL for reference to the upstream xorg driver. v3: Unchanged --- spice-video-dummy/COPYING| 2 + spice-video-dummy/Makefile.am| 31 ++ spice-video-dummy/README.md | 18 + spice-video-dummy/autogen.sh | 17 + spice-video-dummy/configure.ac | 72 spice-video-dummy/src/Makefile.am| 39 ++ spice-video-dummy/src/compat-api.h | 101 + spice-video-dummy/src/dummy.h| 59 +++ spice-video-dummy/src/dummy_cursor.c | 102 + spice-video-dummy/src/dummy_driver.c | 740 +++ 10 files changed, 1181 insertions(+) create mode 100644 spice-video-dummy/COPYING create mode 100644 spice-video-dummy/Makefile.am create mode 100644 spice-video-dummy/README.md create mode 100755 spice-video-dummy/autogen.sh create mode 100644 spice-video-dummy/configure.ac create mode 100644 spice-video-dummy/src/Makefile.am create mode 100644 spice-video-dummy/src/compat-api.h create mode 100644 spice-video-dummy/src/dummy.h create mode 100644 spice-video-dummy/src/dummy_cursor.c create mode 100644 spice-video-dummy/src/dummy_driver.c diff --git a/spice-video-dummy/COPYING b/spice-video-dummy/COPYING new file mode 100644 index 000..3c1ef48 --- /dev/null +++ b/spice-video-dummy/COPYING @@ -0,0 +1,2 @@ +Copyright 2002, SuSE Linux AG, Author: Egbert Eich + diff --git a/spice-video-dummy/Makefile.am b/spice-video-dummy/Makefile.am new file mode 100644 index 000..1a10a1c --- /dev/null +++ b/spice-video-dummy/Makefile.am @@ -0,0 +1,31 @@ +# Copyright 2005 Adam Jackson. +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# on the rights to use, copy, modify, merge, publish, distribute, sub +# license, and/or sell copies of the Software, and to permit persons to whom +# the Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL +# ADAM JACKSON BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER +# IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +SUBDIRS = src +MAINTAINERCLEANFILES = ChangeLog + +.PHONY: ChangeLog + +ChangeLog: + $(CHANGELOG_CMD) + +dist-hook: ChangeLog + +EXTRA_DIST = README.md diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md new file mode 100644 index 000..2a84bc3 --- /dev/null +++ b/spice-video-dummy/README.md @@ -0,0 +1,18 @@ +xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server +-- + +All questions regarding this software should be directed at the +Xorg mailing list: + + https://lists.x.org/mailman/listinfo/xorg + +The master development code repository can be found at: + + https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy + +Please submit bug reports and requests to merge patches there. + +For patch submission instructions, see: + + https://www.x.org/wiki/Development/Documentation/SubmittingPatches + diff --git a/spice-video-dummy/autogen.sh b/spice-video-dummy/autogen.sh new file mode 100755 index 000..9b9f7e7 --- /dev/null +++ b/spice-video-dummy/autogen.sh @@ -0,0 +1,17 @@ +#! /bin/sh + +srcdir=`dirname "$0"` +test -z "$srcdir" && srcdir=. + +ORIGDIR=`pwd` +cd "$srcdir" + +autoreconf -v --install || exit 1 +cd "$ORIGDIR" || exit $? + +git config --local --get format.subjectPrefix >/dev/null 2>&1 || +git config --local format.subjectPrefix "PATCH xf86-video-dummy" + +if test -z "$NOCONFIGURE"; then +exec "$srcdir"/configure "$@" +fi diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac new file mode 100644 index 000..2ca0327 --- /dev/null +++ b/spice-video-dummy/configure.a
[Spice-devel] [PATCH v3 x11spice 4/5] Add a --enable-dummy option to configure.
Provide some basic documentation as to how to use the new spice-video-dummy driver. Signed-off-by: Jeremy White --- v3: Unchanged --- Makefile.am | 3 +++ README | 19 +++ configure.ac| 7 +++ spice-video-dummy/spicedummy.sh | 30 ++ 4 files changed, 59 insertions(+) create mode 100755 spice-video-dummy/spicedummy.sh diff --git a/Makefile.am b/Makefile.am index b9452f6..cd23d81 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,7 @@ SUBDIRS = src +if DUMMY +SUBDIRS += spice-video-dummy +endif rpm: dist rpmbuild -ba --define "_sourcedir $$PWD" src/data/x11spice.spec diff --git a/README b/README index 34e8c1f..3dbee80 100644 --- a/README +++ b/README @@ -75,6 +75,25 @@ password you can use to connect. Refer to the x11spice man page for more details. +Spice Video Dummy Driver + +If you wish to create a 'headless' connection to a server - create an +X session without using the main system display - there is a straight +forward process that is now included with x11spice. + +That is, x11spice now optionally builds an Xorg driver that is based +on the dummy driver and the modesetting driver. This gives a virtual +frame buffer driver which should include modern Mesa capabilities. + +To use this driver, simply add: + + --enable-dummy + +to the configure line. This will build the Xorg driver. A simplistic +example is included showing the general concepts of using this driver. +Refer to the spice-video-dummy/spicedummy.sh bash script for more +information. + Using code coverage (gcov) - diff --git a/configure.ac b/configure.ac index cee7ec3..0acb4d2 100644 --- a/configure.ac +++ b/configure.ac @@ -17,6 +17,13 @@ PKG_CHECK_MODULES(PIXMAN, pixman-1) AM_CONDITIONAL([HAVE_GTEST], [pkg-config --atleast-version=2.38 glib-2.0]) +AC_ARG_ENABLE([dummy], AS_HELP_STRING([--enable-dummy], [Builds the spice-video-dummy driver]), [dummy=true]) +AM_CONDITIONAL([DUMMY], test x$dummy = xtrue) + +AS_IF([test "x$dummy" = "xtrue"], [ + AC_CONFIG_SUBDIRS([spice-video-dummy]) +]) + AC_PROG_SED # To use gcov, make sure you have the appropriate autoconf macors, # and uncomment this line, and the matching one in src/Makefile.am. diff --git a/spice-video-dummy/spicedummy.sh b/spice-video-dummy/spicedummy.sh new file mode 100755 index 000..c83934c --- /dev/null +++ b/spice-video-dummy/spicedummy.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Example script that shows how to launch the spice video driver +# and attach x11spice to it. This is not meant for production +# purposes. + +export DISPLAY=:2 + +password=`hexdump -n2 -e '2/1 "%02x"' /dev/urandom` +zerodir=`dirname $0` +dummydir=`(cd $zerodir; pwd)` +xmodules=`Xorg -showDefaultModulePath 2>&1` + +# Start Xorg +xinit -- `which Xorg` $DISPLAY \ +-modulepath "$dummydir/src/.libs,$xmodules" \ +-config "$dummydir/spicedummy.conf" & +xpid=$! +sleep 1 + +$dummydir/../src/x11spice --password=$password --allow-control & +spicepid=$! + +echo Xorg server started as pid $xpid +echo Spice server started as pid $spicepid +echo You should be able to connect with a spice client now to port 5900, +echo using a password of $password + +wait $spicepid + +kill $xpid -- 2.11.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 x11spice 2/5] Apply Spice style to the spice-video-dummy
Do this using the spice_indent script which uses indent to apply
current Spice styles (as per Frediano Ziglio on 2019-05-01.
Signed-off-by: Jeremy White
---
v2: Original version
v3: Revised to have case indent at 0, cuddle else, fix functions
returning Bool, and preserve Xorg function declaration style
---
doc/spice_indent | 26 +-
spice-video-dummy/src/dummy.h| 9 +-
spice-video-dummy/src/dummy_cursor.c | 20 +-
spice-video-dummy/src/dummy_driver.c | 490 +--
4 files changed, 270 insertions(+), 275 deletions(-)
diff --git a/doc/spice_indent b/doc/spice_indent
index 08595d5..5d7e29a 100755
--- a/doc/spice_indent
+++ b/doc/spice_indent
@@ -5,21 +5,38 @@
# x11spice code largely conform to the Spice Project Coding Style.
# It's preserved here as a form of documentation.
+# Note: Spice is now relaxed about function declarations,
+# and allow either style, e.g.
+# char *func(){ or
+#
+# char *
+# fund(){
+# That requires calling indent with some thoughtfulness
+# By default, it will break procedure types onto newlines
+# if you have a file that does not do that, you may want
+# to add
+# --dont-break-procedure-type
+# to your invocation.
+
+
if [ ! -f "$1" ] ; then
echo Error: specify a filename to process with indent
+echo Usage:
+echo 'spice_indent [ extra-options ]'
exit 1
fi
+fname="$1"
+shift
-indent "$1" \
+indent "$fname" \
--no-space-after-function-call-names \
- --dont-break-procedure-type \
--braces-after-func-def-line \
--braces-on-if-line \
--braces-on-struct-decl-line \
--line-length 100 \
--no-space-after-parentheses \
--no-tabs \
- --case-indentation 4 \
+ --case-indentation 0 \
--tab-size 4\
--indent-label 0\
--indent-level 4\
@@ -28,6 +45,7 @@ indent "$1" \
--space-after-while \
--space-after-casts \
--space-after-if \
+ --cuddle-else \
-T agent_t \
-T SpiceCharDeviceInstance \
-T SpiceChannelEventInfo \
@@ -64,3 +82,5 @@ indent "$1" \
-T xdummy_t \
-T x11spice_server_t \
-T test_t \
+ -T Bool \
+ $@
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..65710ef 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -29,15 +29,13 @@ extern void DUMMYShowCursor(ScrnInfoPtr pScrn);
extern void DUMMYHideCursor(ScrnInfoPtr pScrn);
/* globals */
-typedef struct _color
-{
+typedef struct _color {
int red;
int green;
int blue;
} dummy_colors;
-typedef struct dummyRec
-{
+typedef struct dummyRec {
/* options */
OptionInfoPtr Options;
Bool swCursor;
@@ -50,10 +48,9 @@ typedef struct dummyRec
int cursorFG, cursorBG;
dummy_colors colors[1024];
-Bool(*CreateWindow)() ; /* wrapped CreateWindow */
+Bool (*CreateWindow) ();/* wrapped CreateWindow */
Bool prop;
} DUMMYRec, *DUMMYPtr;
/* The privates of the DUMMY driver */
#define DUMMYPTR(p)((DUMMYPtr)((p)->driverPrivate))
-
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..f38ac53 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -17,7 +17,7 @@ dummyShowCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-dPtr->DummyHWCursorShown = TRUE;
+dPtr->DummyHWCursorShown = TRUE;
}
static void
@@ -26,7 +26,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
- * turn cursor off
+ * turn cursor off
*
*/
dPtr->DummyHWCursorShown = FALSE;
@@ -47,7 +47,7 @@ static void
dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
{
DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
dPtr->cursorFG = fg;
dPtr->cursorBG = bg;
}
@@ -61,11 +61,11 @@ static Bool
dummyUseHWCursor(ScreenPtr pScr, CursorPtr pCurs)
{
DUMMYPtr dPtr = DUMMYPTR(xf86ScreenToScrn(pScr));
-return(!dPtr->swCursor);
+return (!dPtr->swCursor);
}
#if 0
-static unsigned char*
+static unsigned char *
dummyRealizeCursor(xf86CursorInfoPtr infoPtr, CursorPtr pCurs)
{
return NULL;
@@ -79,7 +79,8 @@ DUMMYCursorInit(ScreenPtr pScreen)
xf86CursorInfoPtr infoPtr;
infoPtr = xf86CreateCursorInfoRec();
-if(!infoPtr) return FALSE;
+if (!infoPtr)
+return FALSE;
dPtr->CursorInfo = infoPtr;
@@ -94,9 +95,6 @@ DUMMYCursorInit(ScreenPtr pScreen)
infoPtr->ShowCursor = dummyShowCursor;
infoPtr->UseHWCursor = dummyUseHWCursor;
/* infoPtr->RealizeCursor = dummyRealizeCursor; */
-
-return(xf86InitCursor(pScreen, infoPtr));
-}
-
-
+return (xf86InitCursor(pScreen, infoPtr));
+}
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..46cde9
[Spice-devel] [PATCH v3 x11spice 5/5] Spelling.
From: Henri Verbeet Signed-off-by: Jeremy White --- v3: New --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 0acb4d2..fa23483 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ AS_IF([test "x$dummy" = "xtrue"], [ ]) AC_PROG_SED -# To use gcov, make sure you have the appropriate autoconf macors, +# To use gcov, make sure you have the appropriate autoconf macros, # and uncomment this line, and the matching one in src/Makefile.am. #AX_CODE_COVERAGE() -- 2.11.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 x11spice 3/5] Initial set of revisions to the dummy driver to add DRI and Present.
This makes the dummy driver similar to the mode setting driver.
This work was done by Henri Verbeet, which was in turn based heavily on
the Xorg modesetting driver.
Signed-off-by: Jeremy White
---
v2: Apply Spice style
v3: Refine Spice style, disable console keyboard and mouse in default
xorg.conf, share an inline function to return microseconds,
more visibly disable glamor when the render node is unavailable,
leave dummy.h as dummy.h
---
spice-video-dummy/.gitignore | 9 +
spice-video-dummy/README.md| 15 +-
spice-video-dummy/configure.ac | 12 +-
spice-video-dummy/spicedummy.conf | 30 +++
spice-video-dummy/src/Makefile.am | 16 +-
spice-video-dummy/src/dri2.c | 291 +
spice-video-dummy/src/dummy.h | 21 ++
spice-video-dummy/src/present.c| 102
.../src/{dummy_driver.c => spicedummy_driver.c}| 84 +-
9 files changed, 549 insertions(+), 31 deletions(-)
create mode 100644 spice-video-dummy/.gitignore
create mode 100644 spice-video-dummy/spicedummy.conf
create mode 100644 spice-video-dummy/src/dri2.c
create mode 100644 spice-video-dummy/src/present.c
rename spice-video-dummy/src/{dummy_driver.c => spicedummy_driver.c} (87%)
diff --git a/spice-video-dummy/.gitignore b/spice-video-dummy/.gitignore
new file mode 100644
index 000..9b4b11f
--- /dev/null
+++ b/spice-video-dummy/.gitignore
@@ -0,0 +1,9 @@
+config.h
+config.h.in
+libtool
+ltmain.sh
+stamp-h1
+src/.libs
+*.o
+*.lo
+*.la
diff --git a/spice-video-dummy/README.md b/spice-video-dummy/README.md
index 2a84bc3..3e77e4d 100644
--- a/spice-video-dummy/README.md
+++ b/spice-video-dummy/README.md
@@ -1,18 +1,15 @@
-xf86-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
+spice-video-dummy - virtual/offscreen frame buffer driver for the Xorg X server
--
All questions regarding this software should be directed at the
-Xorg mailing list:
+spice-devel mailing list:
- https://lists.x.org/mailman/listinfo/xorg
+ https://lists.freedesktop.org/mailman/listinfo/spice-devel
-The master development code repository can be found at:
+This driver originated with the dummy driver from:
https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy
-Please submit bug reports and requests to merge patches there.
-
-For patch submission instructions, see:
-
- https://www.x.org/wiki/Development/Documentation/SubmittingPatches
+and modified to be similar to the mode setting driver:
+
https://gitlab.freedesktop.org/xorg/xserver/tree/master/hw/xfree86/drivers/modesetting
diff --git a/spice-video-dummy/configure.ac b/spice-video-dummy/configure.ac
index 2ca0327..e85b7e5 100644
--- a/spice-video-dummy/configure.ac
+++ b/spice-video-dummy/configure.ac
@@ -22,10 +22,10 @@
# Initialize Autoconf
AC_PREREQ([2.60])
-AC_INIT([xf86-video-dummy],
-[0.3.8],
-[https://gitlab.freedesktop.org/xorg/driver/xf86-video-dummy/issues],
-[xf86-video-dummy])
+AC_INIT([spice-video-dummy],
+[1.1],
+[https://gitlab.freedesktop.org/spice/x11spice/issues],
+[spice-video-dummy])
AC_CONFIG_SRCDIR([Makefile.am])
AC_CONFIG_HEADERS([config.h])
AC_CONFIG_AUX_DIR(.)
@@ -61,10 +61,6 @@ PKG_CHECK_MODULES(XORG, [xorg-server >= 1.4.99.901] xproto
fontsproto $REQUIRED_
# Checks for libraries.
-
-DRIVER_NAME=dummy
-AC_SUBST([DRIVER_NAME])
-
AC_CONFIG_FILES([
Makefile
src/Makefile
diff --git a/spice-video-dummy/spicedummy.conf
b/spice-video-dummy/spicedummy.conf
new file mode 100644
index 000..ad2016c
--- /dev/null
+++ b/spice-video-dummy/spicedummy.conf
@@ -0,0 +1,30 @@
+Section "Device"
+Identifier "SpiceDummy"
+Driver "spicedummy"
+# The SWcursor option enables a software cursor. Default false.
+#Option "SWcursor" "false"
+# AccelMethod can be glamor (the default), or none.
+#Option "AccelMethod" "glamor"
+# kmsdev specifies which framebuffer device to use, default
/dev/dri/renderD128
+#Option "kmsdev" "/dev/dri/renderD128"
+VideoRam 16384
+EndSection
+
+Section "InputDevice"
+Identifier "dummy_mouse"
+Option "CorePointer" "true"
+Driver "void"
+EndSection
+
+Section "InputDevice"
+Identifier "dummy_keyboard"
+Option "CoreKeyboard" "true"
+Driver "void"
+EndSection
+
+Section "Screen"
+Identifier "SpiceDummy"
+SubSection "Display"
+Virtual 1920 1200
+EndSubSection
+EndSection
diff --git a/spice-video-dummy/src/Makefile.am
b/spice-video-dummy/src/Makef
Re: [Spice-devel] [PATCH v3 x11spice 2/5] Apply Spice style to the spice-video-dummy
On 5/3/19 3:27 AM, Frediano Ziglio wrote: Do this using the spice_indent script which uses indent to apply current Spice styles (as per Frediano Ziglio on 2019-05-01. Do you miss a close parenthesis? Not a big issue Yep, caught it in the push, thanks. Signed-off-by: Jeremy White --- v2: Original version v3: Revised to have case indent at 0, cuddle else, fix functions returning Bool, and preserve Xorg function declaration style Got a follow up at https://cgit.freedesktop.org/~fziglio/x11spice/log/?h=jw, specifically https://cgit.freedesktop.org/~fziglio/x11spice/commit/?h=jw&id=fe8a37fca56de792099a6bf1ad316d3a4aa22a5f. Alright, you've demonstrated you care a great deal about the particulars of the style ; fixups applied, and pushed. Thanks! Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice] Changes to build on the 32 bit raspberry pi.
Signed-off-by: Jeremy White
---
src/scan.c| 6 +++---
src/session.c | 8
src/spice.c | 18 +++---
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index bb020de..ebadaf8 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -70,7 +70,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
return NULL;
qxl_image = (QXLImage *) (drawable + 1);
-drawable->release_info.id = (uint64_t) spice_create_release(s,
RELEASE_SHMI, shmi);
+drawable->release_info.id = (uint64_t) ((uintptr_t)
spice_create_release(s, RELEASE_SHMI, shmi));
shmi->drawable_ptr = drawable;
drawable->surface_id = 0;
@@ -91,7 +91,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
drawable->u.copy.src_area.bottom = shmi->h;
drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT;
-drawable->u.copy.src_bitmap = (QXLPHYSICAL) qxl_image;
+drawable->u.copy.src_bitmap = (QXLPHYSICAL) ((uintptr_t) qxl_image);
qxl_image->descriptor.id = 0;
qxl_image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
@@ -106,7 +106,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
qxl_image->bitmap.y = shmi->h;
qxl_image->bitmap.stride = shmi->bytes_per_line;
qxl_image->bitmap.palette = 0;
-qxl_image->bitmap.data = (QXLPHYSICAL) shmi->shmaddr;
+qxl_image->bitmap.data = (QXLPHYSICAL) ((uintptr_t) shmi->shmaddr);
return drawable;
}
diff --git a/src/session.c b/src/session.c
index e1b3c00..2476460 100644
--- a/src/session.c
+++ b/src/session.c
@@ -60,13 +60,13 @@ session_t *global_session = NULL;
void free_cursor_queue_item(gpointer data)
{
QXLCursorCmd *ccmd = (QXLCursorCmd *) data;
-spice_free_release((spice_release_t *) ccmd->release_info.id);
+spice_free_release((spice_release_t *) ((uintptr_t)
ccmd->release_info.id));
}
void free_draw_queue_item(gpointer data)
{
QXLDrawable *drawable = (QXLDrawable *) data;
-spice_free_release((spice_release_t *) drawable->release_info.id);
+spice_free_release((spice_release_t *) ((uintptr_t)
drawable->release_info.id));
}
void *session_pop_draw(session_t *session)
@@ -407,10 +407,10 @@ int session_push_cursor_image(session_t *s,
ccmd->type = QXL_CURSOR_SET;
ccmd->u.set.position.x = x + xhot;
ccmd->u.set.position.y = y + yhot;
-ccmd->u.set.shape = (QXLPHYSICAL) cursor;
+ccmd->u.set.shape = (QXLPHYSICAL) ((uintptr_t) cursor);
ccmd->u.set.visible = TRUE;
-ccmd->release_info.id = (uint64_t) spice_create_release(&s->spice,
RELEASE_MEMORY, ccmd);
+ccmd->release_info.id = (uint64_t) ((uintptr_t)
spice_create_release(&s->spice, RELEASE_MEMORY, ccmd));
g_async_queue_push(s->cursor_queue, ccmd);
spice_qxl_wakeup(&s->spice.display_sin);
diff --git a/src/spice.c b/src/spice.c
index 8489b67..75e9880 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -33,6 +33,10 @@
#include
#include
+/* Obtain definitions for PRId64 */
+#define __STDC_FORMAT_MACROS 1
+#include
+
#include "local_spice.h"
#include "x11spice.h"
#include "display.h"
@@ -255,7 +259,7 @@ static int get_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_DRAW;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) drawable;
+cmd->cmd.data = (QXLPHYSICAL) ((uintptr_t) drawable);
return 1;
}
@@ -273,7 +277,7 @@ static int req_cmd_notification(QXLInstance *qin)
static void release_resource(QXLInstance *qin G_GNUC_UNUSED,
struct QXLReleaseInfoExt release_info)
{
-spice_free_release((spice_release_t *) release_info.info->id);
+spice_free_release((spice_release_t *) ((uintptr_t)
release_info.info->id));
}
static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *cmd)
@@ -289,7 +293,7 @@ static int get_cursor_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_CURSOR;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) cursor;
+cmd->cmd.data = (QXLPHYSICAL) ((uintptr_t) cursor);
return 1;
}
@@ -318,8 +322,8 @@ static int flush_resources(QXLInstance *qin G_GNUC_UNUSED)
static void async_complete(QXLInstance *qin G_GNUC_UNUSED, uint64_t cookie)
{
-g_debug("%s: cookie 0x%lx", __FUNCTION__, cookie);
-spice_free_release((spice_release_t *) cookie);
+g_debug("%s: cookie 0x%"PRId64"x", __FUNCTION__, cookie);
+spice_free_release((spice_release_t *) ((uintptr_t) cookie));
}
static void update_area_complete(QXLInstance *qin G_GNUC_UNUSED,
@@ -463,7 +467,7 @@ static int send_monitors_config(spice_t *s, int w, int h)
monitor
[Spice-devel] [PATCH x11spice v2] Changes to build on the 32 bit raspberry pi.
Signed-off-by: Jeremy White
---
v2 Use (u) (u) thing instead of (u) ((u) thing)
Simplify some casting where possible
---
src/scan.c| 6 +++---
src/session.c | 8
src/spice.c | 18 +++---
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index bb020de..aa10a07 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -70,7 +70,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
return NULL;
qxl_image = (QXLImage *) (drawable + 1);
-drawable->release_info.id = (uint64_t) spice_create_release(s,
RELEASE_SHMI, shmi);
+drawable->release_info.id = (uintptr_t) spice_create_release(s,
RELEASE_SHMI, shmi);
shmi->drawable_ptr = drawable;
drawable->surface_id = 0;
@@ -91,7 +91,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
drawable->u.copy.src_area.bottom = shmi->h;
drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT;
-drawable->u.copy.src_bitmap = (QXLPHYSICAL) qxl_image;
+drawable->u.copy.src_bitmap = (uintptr_t) qxl_image;
qxl_image->descriptor.id = 0;
qxl_image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
@@ -106,7 +106,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
qxl_image->bitmap.y = shmi->h;
qxl_image->bitmap.stride = shmi->bytes_per_line;
qxl_image->bitmap.palette = 0;
-qxl_image->bitmap.data = (QXLPHYSICAL) shmi->shmaddr;
+qxl_image->bitmap.data = (uintptr_t) shmi->shmaddr;
return drawable;
}
diff --git a/src/session.c b/src/session.c
index e1b3c00..1e59415 100644
--- a/src/session.c
+++ b/src/session.c
@@ -60,13 +60,13 @@ session_t *global_session = NULL;
void free_cursor_queue_item(gpointer data)
{
QXLCursorCmd *ccmd = (QXLCursorCmd *) data;
-spice_free_release((spice_release_t *) ccmd->release_info.id);
+spice_free_release((spice_release_t *) (uintptr_t) ccmd->release_info.id);
}
void free_draw_queue_item(gpointer data)
{
QXLDrawable *drawable = (QXLDrawable *) data;
-spice_free_release((spice_release_t *) drawable->release_info.id);
+spice_free_release((spice_release_t *) (uintptr_t)
drawable->release_info.id);
}
void *session_pop_draw(session_t *session)
@@ -407,10 +407,10 @@ int session_push_cursor_image(session_t *s,
ccmd->type = QXL_CURSOR_SET;
ccmd->u.set.position.x = x + xhot;
ccmd->u.set.position.y = y + yhot;
-ccmd->u.set.shape = (QXLPHYSICAL) cursor;
+ccmd->u.set.shape = (uintptr_t) cursor;
ccmd->u.set.visible = TRUE;
-ccmd->release_info.id = (uint64_t) spice_create_release(&s->spice,
RELEASE_MEMORY, ccmd);
+ccmd->release_info.id = (uintptr_t) spice_create_release(&s->spice,
RELEASE_MEMORY, ccmd);
g_async_queue_push(s->cursor_queue, ccmd);
spice_qxl_wakeup(&s->spice.display_sin);
diff --git a/src/spice.c b/src/spice.c
index 8489b67..5b95720 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -33,6 +33,10 @@
#include
#include
+/* Obtain definitions for PRId64 */
+#define __STDC_FORMAT_MACROS 1
+#include
+
#include "local_spice.h"
#include "x11spice.h"
#include "display.h"
@@ -255,7 +259,7 @@ static int get_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_DRAW;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) drawable;
+cmd->cmd.data = (uintptr_t) drawable;
return 1;
}
@@ -273,7 +277,7 @@ static int req_cmd_notification(QXLInstance *qin)
static void release_resource(QXLInstance *qin G_GNUC_UNUSED,
struct QXLReleaseInfoExt release_info)
{
-spice_free_release((spice_release_t *) release_info.info->id);
+spice_free_release((spice_release_t *) (uintptr_t) release_info.info->id);
}
static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *cmd)
@@ -289,7 +293,7 @@ static int get_cursor_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_CURSOR;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) cursor;
+cmd->cmd.data = (uintptr_t) cursor;
return 1;
}
@@ -318,8 +322,8 @@ static int flush_resources(QXLInstance *qin G_GNUC_UNUSED)
static void async_complete(QXLInstance *qin G_GNUC_UNUSED, uint64_t cookie)
{
-g_debug("%s: cookie 0x%lx", __FUNCTION__, cookie);
-spice_free_release((spice_release_t *) cookie);
+g_debug("%s: cookie 0x%"PRId64"x", __FUNCTION__, cookie);
+spice_free_release((spice_release_t *) (uintptr_t) cookie);
}
static void update_area_complete(QXLInstance *qin G_GNUC_UNUSED,
@@ -463,7 +467,7 @@ static int send_monitors_config(spice_t *s, int w, int h)
monitors->heads[0].width =
[Spice-devel] [PATCH x11spice v3] Changes to build on the 32 bit raspberry pi.
Signed-off-by: Jeremy White
---
v3 Use the obviously better PRIx64
v2 Use (u) (u) thing instead of (u) ((u) thing)
Simplify some casting where possible
---
src/scan.c| 6 +++---
src/session.c | 8
src/spice.c | 18 +++---
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/scan.c b/src/scan.c
index bb020de..aa10a07 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -70,7 +70,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
return NULL;
qxl_image = (QXLImage *) (drawable + 1);
-drawable->release_info.id = (uint64_t) spice_create_release(s,
RELEASE_SHMI, shmi);
+drawable->release_info.id = (uintptr_t) spice_create_release(s,
RELEASE_SHMI, shmi);
shmi->drawable_ptr = drawable;
drawable->surface_id = 0;
@@ -91,7 +91,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
drawable->u.copy.src_area.bottom = shmi->h;
drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT;
-drawable->u.copy.src_bitmap = (QXLPHYSICAL) qxl_image;
+drawable->u.copy.src_bitmap = (uintptr_t) qxl_image;
qxl_image->descriptor.id = 0;
qxl_image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
@@ -106,7 +106,7 @@ static QXLDrawable *shm_image_to_drawable(spice_t *s,
shm_image_t *shmi, int x,
qxl_image->bitmap.y = shmi->h;
qxl_image->bitmap.stride = shmi->bytes_per_line;
qxl_image->bitmap.palette = 0;
-qxl_image->bitmap.data = (QXLPHYSICAL) shmi->shmaddr;
+qxl_image->bitmap.data = (uintptr_t) shmi->shmaddr;
return drawable;
}
diff --git a/src/session.c b/src/session.c
index e1b3c00..1e59415 100644
--- a/src/session.c
+++ b/src/session.c
@@ -60,13 +60,13 @@ session_t *global_session = NULL;
void free_cursor_queue_item(gpointer data)
{
QXLCursorCmd *ccmd = (QXLCursorCmd *) data;
-spice_free_release((spice_release_t *) ccmd->release_info.id);
+spice_free_release((spice_release_t *) (uintptr_t) ccmd->release_info.id);
}
void free_draw_queue_item(gpointer data)
{
QXLDrawable *drawable = (QXLDrawable *) data;
-spice_free_release((spice_release_t *) drawable->release_info.id);
+spice_free_release((spice_release_t *) (uintptr_t)
drawable->release_info.id);
}
void *session_pop_draw(session_t *session)
@@ -407,10 +407,10 @@ int session_push_cursor_image(session_t *s,
ccmd->type = QXL_CURSOR_SET;
ccmd->u.set.position.x = x + xhot;
ccmd->u.set.position.y = y + yhot;
-ccmd->u.set.shape = (QXLPHYSICAL) cursor;
+ccmd->u.set.shape = (uintptr_t) cursor;
ccmd->u.set.visible = TRUE;
-ccmd->release_info.id = (uint64_t) spice_create_release(&s->spice,
RELEASE_MEMORY, ccmd);
+ccmd->release_info.id = (uintptr_t) spice_create_release(&s->spice,
RELEASE_MEMORY, ccmd);
g_async_queue_push(s->cursor_queue, ccmd);
spice_qxl_wakeup(&s->spice.display_sin);
diff --git a/src/spice.c b/src/spice.c
index 8489b67..8ec0b07 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -33,6 +33,10 @@
#include
#include
+/* Obtain definitions for PRIx64 */
+#define __STDC_FORMAT_MACROS 1
+#include
+
#include "local_spice.h"
#include "x11spice.h"
#include "display.h"
@@ -255,7 +259,7 @@ static int get_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_DRAW;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) drawable;
+cmd->cmd.data = (uintptr_t) drawable;
return 1;
}
@@ -273,7 +277,7 @@ static int req_cmd_notification(QXLInstance *qin)
static void release_resource(QXLInstance *qin G_GNUC_UNUSED,
struct QXLReleaseInfoExt release_info)
{
-spice_free_release((spice_release_t *) release_info.info->id);
+spice_free_release((spice_release_t *) (uintptr_t) release_info.info->id);
}
static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *cmd)
@@ -289,7 +293,7 @@ static int get_cursor_command(QXLInstance *qin, struct
QXLCommandExt *cmd)
cmd->flags = 0;
cmd->cmd.type = QXL_CMD_CURSOR;
cmd->cmd.padding = 0;
-cmd->cmd.data = (QXLPHYSICAL) cursor;
+cmd->cmd.data = (uintptr_t) cursor;
return 1;
}
@@ -318,8 +322,8 @@ static int flush_resources(QXLInstance *qin G_GNUC_UNUSED)
static void async_complete(QXLInstance *qin G_GNUC_UNUSED, uint64_t cookie)
{
-g_debug("%s: cookie 0x%lx", __FUNCTION__, cookie);
-spice_free_release((spice_release_t *) cookie);
+g_debug("%s: cookie %#" PRIx64, __FUNCTION__, cookie);
+spice_free_release((spice_release_t *) (uintptr_t) cookie);
}
static void update_area_complete(QXLInstance *qin G_GNUC_UNUSED,
@@ -463,7 +467,7 @@ static int send_monitors_config(spice_t *s, int w, int h)
moni
Re: [Spice-devel] [PATCH spice-server 00/23] WebSocket support
Series Acked-by: Jeremy White On 6/25/19 11:11 AM, Frediano Ziglio wrote: Updated a longstanding patch for WebSocket support. This includes: - style updates: - types (C99 instead of GLib); - memory allocation (GLib instead of SPICE functions); - support for Windows; - different fixes; - automatic tests using Autobahn Testsuite; - some missing features: - PING/PONG messages; - empty (0 bytes) frames. Frediano Ziglio (22): test-glib-compat: Fix G_PID_FORMAT definition for old systems reds: Fix SSL_CTX_set_ecdh_auto call for some old OpenSSL websocket: Simplify and fix constrain_iov websocket: Move RedsWebSocket to header websocket: Make websocket function more ABI compatibles with RedStream websocket: Make websocket_ack_close static websocket: New interface to create RedsWebSocket websocket: Better encapsulation websocket: Detect and handle some header error websocket: Better variable types websocket: Propagate some variable websocket: Fix updating remaining bytes to write in websocket_write websocket: Avoid possible server crash using websockets websocket: Support correctly protocol values websocket: Handle case if server cannot write the header entirely websocket: Avoids to write close frame in the middle of data websocket: Handle PING and PONG frames test-websocket: Write a test helper to make possible to run Autobahn testsuite websocket: Do not require "Sec-WebSocket-Protocol" header websocket: Handle text data websocket: Handle continuation and 0-size frames ci: Add test for websockets Jeremy White (1): Add support for clients connecting with the WebSocket protocol. .gitlab-ci.yml | 20 + configure.ac | 9 + server/Makefile.am | 2 + server/meson.build | 2 + server/red-stream.c| 56 ++ server/red-stream.h| 2 + server/reds.c | 15 + server/tests/.gitignore| 1 + server/tests/Makefile.am | 6 + server/tests/autobahn-check-report | 18 + server/tests/fuzzingclient.json| 11 + server/tests/meson.build | 1 + server/tests/test-glib-compat.h| 2 +- server/tests/test-websocket.c | 299 +++ server/websocket.c | 796 + server/websocket.h | 43 ++ 16 files changed, 1282 insertions(+), 1 deletion(-) create mode 100755 server/tests/autobahn-check-report create mode 100644 server/tests/fuzzingclient.json create mode 100644 server/tests/test-websocket.c create mode 100644 server/websocket.c create mode 100644 server/websocket.h ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice] Use C99 struct initializiers instead of memset for local structures.
Signed-off-by: Jeremy White
---
src/gui.c| 3 +--
src/listen.c | 3 +--
src/main.c | 4 +---
src/spice.c | 3 +--
4 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/gui.c b/src/gui.c
index 6748f66e..88acf5c9 100644
--- a/src/gui.c
+++ b/src/gui.c
@@ -147,10 +147,9 @@ void session_disconnect_client(session_t *session)
int main(int argc, char *argv[])
{
gui_t gui;
-session_t session;
+session_t session = { 0 };
setlocale(LC_ALL, "");
-memset(&session, 0, sizeof(session));
gui_create(&gui, &session, argc, argv);
gui_run(&gui);
gui_destroy(&gui);
diff --git a/src/listen.c b/src/listen.c
index 1bddf7ed..452fd81f 100644
--- a/src/listen.c
+++ b/src/listen.c
@@ -117,11 +117,10 @@ int listen_parse(const char *listen_spec, char **addr,
int *port_start, int *por
static int try_port(const char *addr, int port)
{
static const int on = 1, off = 0;
-struct addrinfo ai, *res, *e;
+struct addrinfo ai = { 0 }, *res, *e;
char portbuf[33];
int sock, rc;
-memset(&ai, 0, sizeof(ai));
ai.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
ai.ai_socktype = SOCK_STREAM;
ai.ai_family = 0;
diff --git a/src/main.c b/src/main.c
index 7f321af9..f18311c9 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,7 +55,7 @@ int main(int argc, char *argv[])
{
int rc;
-session_t session;
+session_t session = { 0 };
int display_opened = 0;
int spice_started = 0;
@@ -63,8 +63,6 @@ int main(int argc, char *argv[])
int session_created = 0;
int session_started = 0;
-memset(&session, 0, sizeof(session));
-
/*
** Parse arguments
**--*/
diff --git a/src/spice.c b/src/spice.c
index d9666441..430df405 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -474,9 +474,8 @@ static int send_monitors_config(spice_t *s, int w, int h)
int spice_create_primary(spice_t *s, int w, int h, int bytes_per_line, void
*shmaddr)
{
-QXLDevSurfaceCreate surface;
+QXLDevSurfaceCreate surface = { 0 };
-memset(&surface, 0, sizeof(surface));
surface.height = h;
surface.width = w;
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 2/2] Bug fix: a listen specification from the config file was ignored
Signed-off-by: Jeremy White
---
src/options.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/options.c b/src/options.c
index 0d3138d0..cf75e54e 100644
--- a/src/options.c
+++ b/src/options.c
@@ -355,7 +355,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-options->listen = strdup("5900");
+if (!options->listen) {
+options->listen = strdup("5900");
+}
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 1/2] Bug fix: --config= did not work.
Signed-off-by: Jeremy White
---
src/options.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/options.c b/src/options.c
index b7f487c5..0d3138d0 100644
--- a/src/options.c
+++ b/src/options.c
@@ -213,14 +213,34 @@ void options_handle_ssl_file_options(options_t *options,
options->ssl.ciphersuite = string_option(userkey, systemkey, "ssl",
"ciphersuite");
}
+/* In general, we want to parse the config file options before the command line
+** arguments. However, the command line argument to specify a config file is
+** the exception. We manually parse this out now, so we can simplify the
+** flow of control later. */
void options_handle_user_config(int argc, char *argv[], options_t *options)
{
int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
+char *p, *q;
+
+/* getopt long is complex; it supports [-]-config[=]filename */
+for (i = 1; i < argc; i++) {
+p = strstr(argv[i], "--config");
+if (p != argv[i]) {
+p = strstr(argv[i], "-config");
+}
+if (p != argv[i]) {
+continue;
+}
+q = strstr(p, "=");
+if (q) {
+options->user_config_file = strdup(q + 1);
+continue;
+}
+if (i < argc - 1) {
options->user_config_file = strdup(argv[i + 1]);
i++;
}
+}
}
int options_parse_arguments(int argc, char *argv[], options_t *options)
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 1/4] Convert to the use of glib memory routines in options.c.
Signed-off-by: Jeremy White
---
src/options.c | 44
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/src/options.c b/src/options.c
index b7f487c5..a206c92c 100644
--- a/src/options.c
+++ b/src/options.c
@@ -52,10 +52,10 @@ void options_init(options_t *options)
void options_free(options_t *options)
{
-if (options->display) {
-free(options->display);
-options->display = NULL;
-}
+g_free(options->display);
+options->display = NULL;
+g_free(options->listen);
+options->listen = NULL;
g_free(options->spice_password);
options->spice_password = NULL;
@@ -69,10 +69,6 @@ void options_free(options_t *options)
g_free(options->on_disconnect);
options->on_disconnect = NULL;
-if (options->listen)
-free(options->listen);
-options->listen = NULL;
-
g_free(options->user_config_file);
options->user_config_file = NULL;
@@ -159,7 +155,7 @@ static void usage(char *argv0)
int options_handle_ssl(options_t *options, const char *spec)
{
char *save = NULL;
-char *in = strdup(spec);
+char *in = g_strdup(spec);
char *p;
int i = 0;
int rc = 0;
@@ -173,22 +169,22 @@ int options_handle_ssl(options_t *options, const char
*spec)
switch(i) {
case 0:
-options->ssl.ca_cert_file = strdup(p);
+options->ssl.ca_cert_file = g_strdup(p);
break;
case 1:
-options->ssl.certs_file = strdup(p);
+options->ssl.certs_file = g_strdup(p);
break;
case 2:
-options->ssl.private_key_file = strdup(p);
+options->ssl.private_key_file = g_strdup(p);
break;
case 3:
-options->ssl.key_password = strdup(p);
+options->ssl.key_password = g_strdup(p);
break;
case 4:
-options->ssl.dh_key_file = strdup(p);
+options->ssl.dh_key_file = g_strdup(p);
break;
case 5:
-options->ssl.ciphersuite = strdup(p);
+options->ssl.ciphersuite = g_strdup(p);
break;
default:
fprintf(stderr, "Error: invalid ssl specification.");
@@ -197,7 +193,7 @@ int options_handle_ssl(options_t *options, const char *spec)
}
}
-free(in);
+g_free(in);
return rc;
}
@@ -218,7 +214,7 @@ void options_handle_user_config(int argc, char *argv[],
options_t *options)
int i;
for (i = 1; i < argc - 1; i++)
if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = strdup(argv[i + 1]);
+options->user_config_file = g_strdup(argv[i + 1]);
i++;
}
}
@@ -278,11 +274,11 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_PASSWORD:
-options->spice_password = strdup(optarg);
+options->spice_password = g_strdup(optarg);
break;
case OPTION_PASSWORD_FILE:
-options->password_file = strdup(optarg);
+options->password_file = g_strdup(optarg);
break;
case OPTION_CONFIG:
@@ -305,7 +301,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_DISPLAY:
-options->display = strdup(optarg);
+options->display = g_strdup(optarg);
break;
case OPTION_MINIMIZE:
@@ -335,12 +331,12 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-options->listen = strdup("5900");
+options->listen = g_strdup("5900");
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
} else {
-options->listen = strdup(argv[optind]);
+options->listen = g_strdup(argv[optind]);
}
}
@@ -434,7 +430,7 @@ static int process_password_file(options_t *options)
if (p > buf && *(p - 1) == '\n')
*(p - 1) = '\0';
-options->spice_password = strdup(buf);
+options->spice_password = g_strdup(buf);
return rc;
}
@@ -449,7 +445,7 @@ static int generate_password(options_t *options)
if (fd < 0)
return X11SPICE_ERR_OPEN;
-p = options->spice_password = malloc(options->generate_passw
[Spice-devel] [PATCH v2 x11spice 4/4] Bug fix: a listen specification from the config file was ignored
Signed-off-by: Jeremy White
---
src/options.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/options.c b/src/options.c
index 5a55f2c9..a3dd20ac 100644
--- a/src/options.c
+++ b/src/options.c
@@ -339,7 +339,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-options->listen = g_strdup("5900");
+if (options->listen == NULL) {
+options->listen = g_strdup("5900");
+}
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 2/4] Free the SSL and password_file option fields.
Signed-off-by: Jeremy White
---
src/options.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/src/options.c b/src/options.c
index a206c92c..303c07de 100644
--- a/src/options.c
+++ b/src/options.c
@@ -50,6 +50,17 @@ void options_init(options_t *options)
memset(options, 0, sizeof(*options));
}
+void ssl_options_free(ssl_options_t *ssl)
+{
+g_free(ssl->ca_cert_file);
+g_free(ssl->certs_file);
+g_free(ssl->private_key_file);
+g_free(ssl->key_password);
+g_free(ssl->dh_key_file);
+g_free(ssl->ciphersuite);
+*ssl = (ssl_options_t) { 0 };
+}
+
void options_free(options_t *options)
{
g_free(options->display);
@@ -57,8 +68,12 @@ void options_free(options_t *options)
g_free(options->listen);
options->listen = NULL;
+ssl_options_free(&options->ssl);
+
g_free(options->spice_password);
options->spice_password = NULL;
+g_free(options->password_file);
+options->password_file = NULL;
g_free(options->virtio_path);
options->virtio_path = NULL;
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 3/4] Bug fix: support --config= semantics.
Instead of trying to parse argv ourselves, just reuse getopt_long_only.
Signed-off-by: Jeremy White
---
src/main.c| 8 +---
src/options.c | 50 --
src/options.h | 2 +-
3 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..72aba5a8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,11 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
-rc = options_parse_arguments(argc, argv, &session.options);
+rc = options_handle_user_config(argc, argv, &session.options);
+if (rc == 0) {
+options_from_config(&session.options);
+rc = options_parse_arguments(argc, argv, &session.options);
+}
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index 303c07de..5a55f2c9 100644
--- a/src/options.c
+++ b/src/options.c
@@ -224,16 +224,6 @@ void options_handle_ssl_file_options(options_t *options,
options->ssl.ciphersuite = string_option(userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -264,6 +254,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -297,7 +288,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_CONFIG:
-/* This was handled previously; we can ignore */
+if (options->user_config_file)
+g_free(options->user_config_file);
+options->user_config_file = g_strdup(optarg);
break;
case OPTION_SSL:
@@ -358,6 +351,21 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
return rc;
}
+/* The idea is to have command line arguments run after all other
+** configuration, with one exception - we want to grab any
+** user specified configuration file first. */
+int options_handle_user_config(int argc, char *argv[], options_t *options)
+{
+options_t tmp_options = { 0 };
+int rc;
+
+rc = options_parse_arguments(argc, argv, &tmp_options);
+options->user_config_file = tmp_options.user_config_file;
+tmp_options.user_config_file = NULL;
+options_free(&tmp_options);
+return rc;
+}
+
void options_from_config(options_t *options)
{
GKeyFile *userkey = g_key_file_new();
@@ -517,11 +525,25 @@ int options_impossible_config(options_t *options)
int main(int argc, char *argv[])
{
options_t options;
+int rc;
options_init(&options);
-options_parse_arguments(argc, argv, &options);
-options_from_config(&options);
-g_message("Options parsed");
+rc = options_handle_user_config(argc, argv, &options);
+if (rc == 0) {
+options_from_config(&options);
+rc = options_parse_arguments(argc, argv, &options);
+}
+if (rc == 0) {
+g_message("Options parsed");
+rc = options_process_io(&options);
+if (rc == 0)
+g_message("Options io run parsed");
+else
+g_message("Options io failed");
+}
+else {
+g_message("Options parse failed");
+}
options_free(&options);
}
#endif
diff --git a/src/options.h b/src/options.h
index 6155984b..162643ac 100644
--- a/src/options.h
+++ b/src/options.h
@@ -73,7 +73,7 @@ typedef struct {
** Prototypes
**--*/
void options_init(options_t *options);
-void options_handle_user_config(int argc, char *argv[], options_t *options);
+int options_handle_user_config(int argc, char *argv[], options_t *options);
int options_parse_arguments(int argc, char *argv[], options_t *options);
int options_process_io(options_t *options);
void options_free(options_t *options);
--
2.11.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 0/4] Resend two small fixes
v2: Use glib memory routines with options.c Fix a memory use bug in options Just call getopt_long twice, rather than trying to do a poor implementation For the listen bug fix, use == NULL, and g_strdup ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice] Fix a bug in the tests.
We were overly dependent on timing for success; now we deliberately wait for our GC and drawing to finish. Signed-off-by: Jeremy White --- src/tests/xcb.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/tests/xcb.c b/src/tests/xcb.c index 6be4a69b..709cdfd4 100644 --- a/src/tests/xcb.c +++ b/src/tests/xcb.c @@ -82,6 +82,8 @@ int xcb_draw_grid(const char *display) xcb_gcontext_t red_fg; xcb_gcontext_t green_fg; +xcb_void_cookie_t cookie; + xcb_rectangle_t red_rectangles[32]; xcb_rectangle_t green_rectangles[32]; @@ -96,19 +98,24 @@ int xcb_draw_grid(const char *display) red_fg = xcb_generate_id(c); lookup_color(c, screen, "red", &pixels[0]); pixels[1] = 0; -xcb_create_gc(c, red_fg, screen->root, XCB_GC_FOREGROUND, pixels); +cookie = xcb_create_gc_checked(c, red_fg, screen->root, XCB_GC_FOREGROUND, pixels); +xcb_request_check(c, cookie); green_fg = xcb_generate_id(c); lookup_color(c, screen, "green", &pixels[0]); pixels[1] = 0; -xcb_create_gc(c, green_fg, screen->root, XCB_GC_FOREGROUND, pixels); +cookie = xcb_create_gc_checked(c, green_fg, screen->root, XCB_GC_FOREGROUND, pixels); +xcb_request_check(c, cookie); create_rectangles(red_rectangles, green_rectangles, screen->width_in_pixels, screen->height_in_pixels); /* We draw the rectangles */ -xcb_poly_fill_rectangle_checked(c, screen->root, red_fg, 32, red_rectangles); -xcb_poly_fill_rectangle_checked(c, screen->root, green_fg, 32, green_rectangles); +cookie = xcb_poly_fill_rectangle_checked(c, screen->root, red_fg, 32, red_rectangles); +xcb_request_check(c, cookie); + +cookie = xcb_poly_fill_rectangle_checked(c, screen->root, green_fg, 32, green_rectangles); +xcb_request_check(c, cookie); /* We flush the request */ xcb_flush(c); -- 2.11.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v3 x11spice 1/4] Convert to the use of glib memory routines in options.c.
Signed-off-by: Jeremy White
---
v3: Updates only 1/4; don't check g_malloc return.
---
src/options.c | 46 --
1 file changed, 20 insertions(+), 26 deletions(-)
diff --git a/src/options.c b/src/options.c
index b7f487c5..e9d33f24 100644
--- a/src/options.c
+++ b/src/options.c
@@ -52,10 +52,10 @@ void options_init(options_t *options)
void options_free(options_t *options)
{
-if (options->display) {
-free(options->display);
-options->display = NULL;
-}
+g_free(options->display);
+options->display = NULL;
+g_free(options->listen);
+options->listen = NULL;
g_free(options->spice_password);
options->spice_password = NULL;
@@ -69,10 +69,6 @@ void options_free(options_t *options)
g_free(options->on_disconnect);
options->on_disconnect = NULL;
-if (options->listen)
-free(options->listen);
-options->listen = NULL;
-
g_free(options->user_config_file);
options->user_config_file = NULL;
@@ -159,7 +155,7 @@ static void usage(char *argv0)
int options_handle_ssl(options_t *options, const char *spec)
{
char *save = NULL;
-char *in = strdup(spec);
+char *in = g_strdup(spec);
char *p;
int i = 0;
int rc = 0;
@@ -173,22 +169,22 @@ int options_handle_ssl(options_t *options, const char
*spec)
switch(i) {
case 0:
-options->ssl.ca_cert_file = strdup(p);
+options->ssl.ca_cert_file = g_strdup(p);
break;
case 1:
-options->ssl.certs_file = strdup(p);
+options->ssl.certs_file = g_strdup(p);
break;
case 2:
-options->ssl.private_key_file = strdup(p);
+options->ssl.private_key_file = g_strdup(p);
break;
case 3:
-options->ssl.key_password = strdup(p);
+options->ssl.key_password = g_strdup(p);
break;
case 4:
-options->ssl.dh_key_file = strdup(p);
+options->ssl.dh_key_file = g_strdup(p);
break;
case 5:
-options->ssl.ciphersuite = strdup(p);
+options->ssl.ciphersuite = g_strdup(p);
break;
default:
fprintf(stderr, "Error: invalid ssl specification.");
@@ -197,7 +193,7 @@ int options_handle_ssl(options_t *options, const char *spec)
}
}
-free(in);
+g_free(in);
return rc;
}
@@ -218,7 +214,7 @@ void options_handle_user_config(int argc, char *argv[],
options_t *options)
int i;
for (i = 1; i < argc - 1; i++)
if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = strdup(argv[i + 1]);
+options->user_config_file = g_strdup(argv[i + 1]);
i++;
}
}
@@ -278,11 +274,11 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_PASSWORD:
-options->spice_password = strdup(optarg);
+options->spice_password = g_strdup(optarg);
break;
case OPTION_PASSWORD_FILE:
-options->password_file = strdup(optarg);
+options->password_file = g_strdup(optarg);
break;
case OPTION_CONFIG:
@@ -305,7 +301,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_DISPLAY:
-options->display = strdup(optarg);
+options->display = g_strdup(optarg);
break;
case OPTION_MINIMIZE:
@@ -335,12 +331,12 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-options->listen = strdup("5900");
+options->listen = g_strdup("5900");
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
} else {
-options->listen = strdup(argv[optind]);
+options->listen = g_strdup(argv[optind]);
}
}
@@ -434,7 +430,7 @@ static int process_password_file(options_t *options)
if (p > buf && *(p - 1) == '\n')
*(p - 1) = '\0';
-options->spice_password = strdup(buf);
+options->spice_password = g_strdup(buf);
return rc;
}
@@ -449,9 +445,7 @@ static int generate_password(options_t *options)
if (fd < 0)
return X11SPICE_ERR_OPEN;
-p = options-&g
Re: [Spice-devel] [PATCH v2 x11spice 1/4] Convert to the use of glib memory routines in options.c.
On 7/22/19 5:16 AM, Frediano Ziglio wrote: Signed-off-by: Jeremy White I didn't expected you would change everything :-) :-). Well, I appreciate the review, so when the suggestions are easy, I try to be responsive. -p = options->spice_password = malloc(options->generate_password + 1); +p = options->spice_password = g_malloc(options->generate_password + 1); if (!p) g_malloc never returns NULL. Do you want to remove the check and accept the abort() or use g_try_malloc ? I suppose the first giving the above code and the fact that many old strdup assumed strdup never returned NULL. Yeah, abort is perfectly fine. I resent this patch. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 x11spice 3/4] Bug fix: support --config= semantics.
On 7/23/19 11:27 AM, Frediano Ziglio wrote:
Instead of trying to parse argv ourselves, just reuse getopt_long_only.
Signed-off-by: Jeremy White
This patch looks a bit long compared to the job is doing.
What is the expected behaviour mixing --config and other options?
It looks like the configuration file should be always read first, all
other options, before or after --config should override the configuration
file. Is it right?
The desired behavior is that the command line options override anything
provided by the configuration files.
If a configuration file is provided on the command line, then we process
only that file. If no configuration file is given, then a config file
in the users home directory overrides a config file in the system location.
Why don't you call options_parse_arguments and if you find the
configuration file you then call options_handle_user_config and
options_parse_arguments again to just override the options loaded
from the file?
That's fundamentally what my code does, with a stop to free data from
the arguments along the way. The str_replace semantic would remove the
need for that extra step.
Was testing something like
https://cgit.freedesktop.org/~fziglio/x11spice/commit/?h=options&id=3595c4f06c0df5056e4cb65367a1de72b0c66056
with
https://cgit.freedesktop.org/~fziglio/x11spice/commit/?h=options&id=ef824f30a1a91ee77f44be7d86861535f7323b0a
(on top of your patches)
I had a look. The str_replace semantic is a clear improvement, but is
largely separate from this patch set. You also caught an extraneous
NULL check I missed :-/.
Your code, as written, does not handle the default configuration files.
To do that, you'd essentially have to trigger the 'again' stanza later.
The functional result of the code would be identical to what I wrote.
And, to be honest, I don't find that any more clear than the expression
I submitted in this patch.
I'll use your str_replace semantic and revise this once again and see if
it makes sense to you then.
Cheers,
Jeremy
---
src/main.c| 8 +---
src/options.c | 50 --
src/options.h | 2 +-
3 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..72aba5a8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,11 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
-rc = options_parse_arguments(argc, argv, &session.options);
+rc = options_handle_user_config(argc, argv, &session.options);
+if (rc == 0) {
+options_from_config(&session.options);
+rc = options_parse_arguments(argc, argv, &session.options);
+}
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index 303c07de..5a55f2c9 100644
--- a/src/options.c
+++ b/src/options.c
@@ -224,16 +224,6 @@ void options_handle_ssl_file_options(options_t *options,
options->ssl.ciphersuite = string_option(userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config")
== 0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -264,6 +254,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -297,7 +288,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_CONFIG:
-/* This was handled previously; we can ignore */
+if (options->user_config_file)
+g_free(options->user_config_file);
+options->user_config_file = g_strdup(optarg);
break;
case OPTION_SSL:
@@ -358,6 +351,21 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
return rc;
}
+/* The idea is to have command line arguments run after all other
+** configuration, with one exception - we want to grab any
+** user specified configuration file first. */
+int options_handle_user_config(int argc, char *argv[], options_t *options)
+{
+options_t tmp_options = { 0 };
+int rc;
+
+rc = options_par
[Spice-devel] [PATCH v4 x11spice 1/3] Convert to the use of glib memory routines in options.c.
Also be sure that when replacing an option string, we free it first,
and free the SSL and password_file option fields.
Signed-off-by: Jeremy White
---
v4: Consolidate a few patches into one
Use a suggestion from Freddy to free memory on replace
Use that to simplify the expression of argument procesing further
Bring in the test code written by Freddy
src/options.c | 125 +-
1 file changed, 62 insertions(+), 63 deletions(-)
diff --git a/src/options.c b/src/options.c
index b7f487c5..bc7cd631 100644
--- a/src/options.c
+++ b/src/options.c
@@ -45,43 +45,46 @@
#include
#endif
+static void str_replace(char **dest, const char *src)
+{
+g_free(*dest);
+*dest = src ? g_strdup(src) : NULL;
+}
+
void options_init(options_t *options)
{
memset(options, 0, sizeof(*options));
}
-void options_free(options_t *options)
+static void ssl_options_free(ssl_options_t *ssl)
{
-if (options->display) {
-free(options->display);
-options->display = NULL;
-}
-
-g_free(options->spice_password);
-options->spice_password = NULL;
-
-g_free(options->virtio_path);
-options->virtio_path = NULL;
-g_free(options->uinput_path);
-options->uinput_path = NULL;
-g_free(options->on_connect);
-options->on_connect = NULL;
-g_free(options->on_disconnect);
-options->on_disconnect = NULL;
-
-if (options->listen)
-free(options->listen);
-options->listen = NULL;
-
-g_free(options->user_config_file);
-options->user_config_file = NULL;
+str_replace(&ssl->ca_cert_file, NULL);
+str_replace(&ssl->certs_file, NULL);
+str_replace(&ssl->private_key_file, NULL);
+str_replace(&ssl->key_password, NULL);
+str_replace(&ssl->dh_key_file, NULL);
+str_replace(&ssl->ciphersuite, NULL);
+}
-g_free(options->system_config_file);
-options->system_config_file = NULL;
+void options_free(options_t *options)
+{
+str_replace(&options->display, NULL);
+str_replace(&options->listen, NULL);
+
+ssl_options_free(&options->ssl);
+str_replace(&options->spice_password, NULL);
+str_replace(&options->password_file, NULL);
+
+str_replace(&options->virtio_path, NULL);
+str_replace(&options->uinput_path, NULL);
+str_replace(&options->on_connect, NULL);
+str_replace(&options->on_disconnect, NULL);
+str_replace(&options->user_config_file, NULL);
+str_replace(&options->system_config_file, NULL);
}
-static gchar *string_option(GKeyFile *u, GKeyFile *s, const gchar *section,
const gchar *key)
+static void string_option(gchar **dest, GKeyFile *u, GKeyFile *s, const gchar
*section, const gchar *key)
{
gchar *ret = NULL;
GError *error = NULL;
@@ -93,7 +96,8 @@ static gchar *string_option(GKeyFile *u, GKeyFile *s, const
gchar *section, cons
if (error)
g_error_free(error);
-return ret;
+g_free(*dest);
+*dest = ret;
}
static gint int_option(GKeyFile *u, GKeyFile *s, const gchar *section, const
gchar *key)
@@ -159,36 +163,33 @@ static void usage(char *argv0)
int options_handle_ssl(options_t *options, const char *spec)
{
char *save = NULL;
-char *in = strdup(spec);
+char *in = g_strdup(spec);
char *p;
int i = 0;
int rc = 0;
-if (!in)
-return X11SPICE_ERR_MALLOC;
-
for (p = strtok_r(in, ",", &save); p; p = strtok_r(NULL, ",", &save), i++)
{
if (strlen(p) == 0)
continue;
switch(i) {
case 0:
-options->ssl.ca_cert_file = strdup(p);
+str_replace(&options->ssl.ca_cert_file, p);
break;
case 1:
-options->ssl.certs_file = strdup(p);
+str_replace(&options->ssl.certs_file, p);
break;
case 2:
-options->ssl.private_key_file = strdup(p);
+str_replace(&options->ssl.private_key_file, p);
break;
case 3:
-options->ssl.key_password = strdup(p);
+str_replace(&options->ssl.key_password, p);
break;
case 4:
-options->ssl.dh_key_file = strdup(p);
+str_replace(&options->ssl.dh_key_file, p);
break;
case 5:
-options->ssl.ciphersuite = strdup(p);
+str_replace(&options->ssl.ciphersuite, p);
break;
default:
fprintf(stderr, "Error: invalid ssl specification.");
@@ -197,7 +198,7 @@ int options_handle_ssl(options_t *options, const char *spec)
}
}
-free(i
[Spice-devel] [PATCH v4 x11spice 2/3] Simplify the expression of option handling.
This fixes a bug with --config= handling.
Add expanded test code courtesy of Frediano Ziglio .
Signed-off-by: Jeremy White
---
src/main.c| 8 +++--
src/options.c | 93 ++-
src/options.h | 1 -
3 files changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..da6d4882 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,13 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
rc = options_parse_arguments(argc, argv, &session.options);
+if (rc == 0) {
+options_from_config(&session.options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, &session.options);
+}
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index bc7cd631..6620b853 100644
--- a/src/options.c
+++ b/src/options.c
@@ -214,16 +214,6 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-str_replace(&options->user_config_file, argv[i + 1]);
-i++;
-}
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -254,6 +244,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -287,7 +278,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
break;
case OPTION_CONFIG:
-/* This was handled previously; we can ignore */
+str_replace(&options->user_config_file, optarg);
break;
case OPTION_SSL:
@@ -501,15 +492,85 @@ int options_impossible_config(options_t *options)
return 1;
}
-#if defined(OPTIONS_MAIN)
-int main(int argc, char *argv[])
+#if defined(MAIN_TEST)
+static int test_load_options(options_t *options, int argc, char **argv)
+{
+int rc;
+rc = options_parse_arguments(argc, argv, options);
+if (rc == 0) {
+options_from_config(options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, options);
+}
+if (rc == 0) {
+rc = options_process_io(options);
+}
+return rc;
+}
+
+static void test(const char *expected, char **argv)
{
options_t options;
+int argc = 0;
+int rc;
+char buf[1000];
+
+while (argv[argc]) {
+++argc;
+}
options_init(&options);
-options_parse_arguments(argc, argv, &options);
-options_from_config(&options);
-g_message("Options parsed");
+rc = test_load_options(&options, argc, argv);
+if (rc == 0) {
+snprintf(buf, sizeof(buf), "display %s allow_control %d listen %s
ca_file %s",
+ options.display, options.allow_control, options.listen,
options.ssl.ca_cert_file);
+if (strcmp(buf, expected) != 0) {
+fprintf(stderr, "Unexpected results:\n\t%s\n%s\n",
+buf, expected);
+}
+}
+else {
+fprintf(stderr, "Unable to load options, rc %d\n", rc);
+}
options_free(&options);
}
+
+#define TEST(exp, ...) do { \
+char *argv[] = { "PROGRAM", __VA_ARGS__, NULL }; \
+test(exp, argv); \
+} while(0)
+
+int main(int argc, char **argv)
+{
+FILE *f = fopen("test.conf", "w");
+
+if (argc > 1) {
+options_t options;
+options_init(&options);
+printf("test_load_options returns %d\n",
+test_load_options(&options, argc, argv));
+options_free(&options);
+}
+
+fprintf(f,
"[spice]\ndisplay=config_display\nlisten=8765\nallow-control=true");
+fclose(f);
+
+TEST("display (null) allow_control 0 listen 5900 ca_file (null)",
+ "--hide");
+TEST("display (null) allow_control 0 listen 1234 ca_file (null)",
+ "1234");
+TEST("display config_
[Spice-devel] [PATCH v4 x11spice 3/3] Bug fix: a listen specification from the config file was ignored
Signed-off-by: Jeremy White
---
src/options.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/options.c b/src/options.c
index 6620b853..64e6ac95 100644
--- a/src/options.c
+++ b/src/options.c
@@ -327,7 +327,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-str_replace(&options->listen, "5900");
+if (options->listen == NULL) {
+str_replace(&options->listen, "5900");
+}
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] PATCH fixup1 for x11spice 0/3
Frediano and I had a slight collision in patch processing. This set of three patches should build on the patches he committed to bring the tree to the state intended by the last patch series. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH fixup1 x11spice 1/3] Simplify string options and make sure they are freed when replaced.
This is largely the work of Frediano Ziglio .
Signed-off-by: Jeremy White
---
src/options.c | 118 +++---
1 file changed, 54 insertions(+), 64 deletions(-)
diff --git a/src/options.c b/src/options.c
index 6c6ecb4e..cff4ac17 100644
--- a/src/options.c
+++ b/src/options.c
@@ -45,54 +45,46 @@
#include
#endif
+static void str_replace(char **dest, const char *src)
+{
+g_free(*dest);
+*dest = src ? g_strdup(src) : NULL;
+}
+
void options_init(options_t *options)
{
memset(options, 0, sizeof(*options));
}
-void ssl_options_free(ssl_options_t *ssl)
+static void ssl_options_free(ssl_options_t *ssl)
{
-g_free(ssl->ca_cert_file);
-g_free(ssl->certs_file);
-g_free(ssl->private_key_file);
-g_free(ssl->key_password);
-g_free(ssl->dh_key_file);
-g_free(ssl->ciphersuite);
-*ssl = (ssl_options_t) { 0 };
+str_replace(&ssl->ca_cert_file, NULL);
+str_replace(&ssl->certs_file, NULL);
+str_replace(&ssl->private_key_file, NULL);
+str_replace(&ssl->key_password, NULL);
+str_replace(&ssl->dh_key_file, NULL);
+str_replace(&ssl->ciphersuite, NULL);
}
void options_free(options_t *options)
{
-g_free(options->display);
-options->display = NULL;
-g_free(options->listen);
-options->listen = NULL;
+str_replace(&options->display, NULL);
+str_replace(&options->listen, NULL);
ssl_options_free(&options->ssl);
-
-g_free(options->spice_password);
-options->spice_password = NULL;
-g_free(options->password_file);
-options->password_file = NULL;
-
-g_free(options->virtio_path);
-options->virtio_path = NULL;
-g_free(options->uinput_path);
-options->uinput_path = NULL;
-g_free(options->on_connect);
-options->on_connect = NULL;
-g_free(options->on_disconnect);
-options->on_disconnect = NULL;
-
-g_free(options->user_config_file);
-options->user_config_file = NULL;
-
-g_free(options->system_config_file);
-options->system_config_file = NULL;
+str_replace(&options->spice_password, NULL);
+str_replace(&options->password_file, NULL);
+
+str_replace(&options->virtio_path, NULL);
+str_replace(&options->uinput_path, NULL);
+str_replace(&options->on_connect, NULL);
+str_replace(&options->on_disconnect, NULL);
+str_replace(&options->user_config_file, NULL);
+str_replace(&options->system_config_file, NULL);
}
-static gchar *string_option(GKeyFile *u, GKeyFile *s, const gchar *section,
const gchar *key)
+static void string_option(gchar **dest, GKeyFile *u, GKeyFile *s, const gchar
*section, const gchar *key)
{
gchar *ret = NULL;
GError *error = NULL;
@@ -104,7 +96,8 @@ static gchar *string_option(GKeyFile *u, GKeyFile *s, const
gchar *section, cons
if (error)
g_error_free(error);
-return ret;
+g_free(*dest);
+*dest = ret;
}
static gint int_option(GKeyFile *u, GKeyFile *s, const gchar *section, const
gchar *key)
@@ -175,31 +168,28 @@ int options_handle_ssl(options_t *options, const char
*spec)
int i = 0;
int rc = 0;
-if (!in)
-return X11SPICE_ERR_MALLOC;
-
for (p = strtok_r(in, ",", &save); p; p = strtok_r(NULL, ",", &save), i++)
{
if (strlen(p) == 0)
continue;
switch(i) {
case 0:
-options->ssl.ca_cert_file = g_strdup(p);
+str_replace(&options->ssl.ca_cert_file, p);
break;
case 1:
-options->ssl.certs_file = g_strdup(p);
+str_replace(&options->ssl.certs_file, p);
break;
case 2:
-options->ssl.private_key_file = g_strdup(p);
+str_replace(&options->ssl.private_key_file, p);
break;
case 3:
-options->ssl.key_password = g_strdup(p);
+str_replace(&options->ssl.key_password, p);
break;
case 4:
-options->ssl.dh_key_file = g_strdup(p);
+str_replace(&options->ssl.dh_key_file, p);
break;
case 5:
-options->ssl.ciphersuite = g_strdup(p);
+str_replace(&options->ssl.ciphersuite, p);
break;
default:
fprintf(stderr, "Error: invalid ssl specification.");
@@ -216,12 +206,12 @@ void options_handle_ssl_file_options(options_t *options,
GKeyFile *userkey, GKeyFile *systemkey)
{
options->ssl.enabled = bool_option(userkey, systemkey, "ssl", "enabled");
-options->
[Spice-devel] [PATCH fixup1 x11spice 3/3] Bug fix: a listen specification from the config file was ignored
Signed-off-by: Jeremy White
---
src/options.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/options.c b/src/options.c
index 6620b853..64e6ac95 100644
--- a/src/options.c
+++ b/src/options.c
@@ -327,7 +327,9 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
if (rc == 0) {
if (optind >= argc) {
/* Default */
-str_replace(&options->listen, "5900");
+if (options->listen == NULL) {
+str_replace(&options->listen, "5900");
+}
} else if (optind < (argc - 1)) {
fprintf(stderr, "Error: too many arguments\n");
rc = X11SPICE_ERR_BADARGS;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH fixup1 x11spice 2/3] Simplify the expression of argument parsing.
This fixes a bug with --config=handling.
This also includes test code courtesy of Frediano Ziglio .
Signed-off-by: Jeremy White
---
src/main.c| 8 +++--
src/options.c | 91 ++-
src/options.h | 1 -
3 files changed, 82 insertions(+), 18 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..da6d4882 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,13 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
rc = options_parse_arguments(argc, argv, &session.options);
+if (rc == 0) {
+options_from_config(&session.options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, &session.options);
+}
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index cff4ac17..6620b853 100644
--- a/src/options.c
+++ b/src/options.c
@@ -214,16 +214,6 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -254,6 +244,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -501,15 +492,85 @@ int options_impossible_config(options_t *options)
return 1;
}
-#if defined(OPTIONS_MAIN)
-int main(int argc, char *argv[])
+#if defined(MAIN_TEST)
+static int test_load_options(options_t *options, int argc, char **argv)
+{
+int rc;
+rc = options_parse_arguments(argc, argv, options);
+if (rc == 0) {
+options_from_config(options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, options);
+}
+if (rc == 0) {
+rc = options_process_io(options);
+}
+return rc;
+}
+
+static void test(const char *expected, char **argv)
{
options_t options;
+int argc = 0;
+int rc;
+char buf[1000];
+
+while (argv[argc]) {
+++argc;
+}
options_init(&options);
-options_parse_arguments(argc, argv, &options);
-options_from_config(&options);
-g_message("Options parsed");
+rc = test_load_options(&options, argc, argv);
+if (rc == 0) {
+snprintf(buf, sizeof(buf), "display %s allow_control %d listen %s
ca_file %s",
+ options.display, options.allow_control, options.listen,
options.ssl.ca_cert_file);
+if (strcmp(buf, expected) != 0) {
+fprintf(stderr, "Unexpected results:\n\t%s\n%s\n",
+buf, expected);
+}
+}
+else {
+fprintf(stderr, "Unable to load options, rc %d\n", rc);
+}
options_free(&options);
}
+
+#define TEST(exp, ...) do { \
+char *argv[] = { "PROGRAM", __VA_ARGS__, NULL }; \
+test(exp, argv); \
+} while(0)
+
+int main(int argc, char **argv)
+{
+FILE *f = fopen("test.conf", "w");
+
+if (argc > 1) {
+options_t options;
+options_init(&options);
+printf("test_load_options returns %d\n",
+test_load_options(&options, argc, argv));
+options_free(&options);
+}
+
+fprintf(f,
"[spice]\ndisplay=config_display\nlisten=8765\nallow-control=true");
+fclose(f);
+
+TEST("display (null) allow_control 0 listen 5900 ca_file (null)",
+ "--hide");
+TEST("display (null) allow_control 0 listen 1234 ca_file (null)",
+ "1234");
+TEST("display config_display allow_control 1 listen 8765 ca_file (null)",
+ "--config=test.conf");
+TEST("display config_display allow_control 1 listen 123 ca_file (null)",
+ "--config=test.conf", "123");
+TEST("display config_display allow_control 0 listen 123 ca_file (null)",
+ &q
Re: [Spice-devel] [PATCH fixup1 x11spice 2/3] Simplify the expression of argument parsing.
On 7/24/19 9:59 AM, Frediano Ziglio wrote:
This fixes a bug with --config=handling.
This also includes test code courtesy of Frediano Ziglio
.
This was very experiment patch, just worked.
I would turn it in a real test as there's a tests directory.
It seems is using just exported APIs so maybe would be better
to put a new test (executable) in tests directory having a
main function and calling/linking options.c.
That feels like serious overkill to me. This code is essentially a
utility to the author to give a hook to run valgrind against, it's not
intended as production test code. Perhaps I'll strip it out to avoid
confusion, and just leave it in a private branch.
Signed-off-by: Jeremy White
---
src/main.c| 8 +++--
src/options.c | 91 ++-
src/options.h | 1 -
3 files changed, 82 insertions(+), 18 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..da6d4882 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,13 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
rc = options_parse_arguments(argc, argv, &session.options);
+if (rc == 0) {
+options_from_config(&session.options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, &session.options);
+}
I personally find a bit complicated API to use, a simple call to
a single function would be IMHO better.
Sure.
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index cff4ac17..6620b853 100644
--- a/src/options.c
+++ b/src/options.c
@@ -214,16 +214,6 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config")
== 0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -254,6 +244,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -501,15 +492,85 @@ int options_impossible_config(options_t *options)
return 1;
}
-#if defined(OPTIONS_MAIN)
What was this OPTIONS_MAIN ?
That was the previous test code.
-int main(int argc, char *argv[])
+#if defined(MAIN_TEST)
+static int test_load_options(options_t *options, int argc, char **argv)
+{
+int rc;
+rc = options_parse_arguments(argc, argv, options);
+if (rc == 0) {
+options_from_config(options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, options);
+}
quite boring that every time you need to use options_parse_arguments
you have to repeat these step.
+if (rc == 0) {
+rc = options_process_io(options);
Is this always mandatory? Can the user of these API avoid to call it?
The point of this was to exercise as much of the code path as possible,
as an aid to running it under valgrind.
+}
+return rc;
+}
+
+static void test(const char *expected, char **argv)
{
options_t options;
+int argc = 0;
+int rc;
+char buf[1000];
+
+while (argv[argc]) {
+++argc;
+}
options_init(&options);
-options_parse_arguments(argc, argv, &options);
-options_from_config(&options);
-g_message("Options parsed");
+rc = test_load_options(&options, argc, argv);
+if (rc == 0) {
+snprintf(buf, sizeof(buf), "display %s allow_control %d listen %s
ca_file %s",
+ options.display, options.allow_control, options.listen,
options.ssl.ca_cert_file);
+if (strcmp(buf, expected) != 0) {
+fprintf(stderr, "Unexpected results:\n\t%s\n%s\n",
+buf, expected);
+}
+}
+else {
+fprintf(stderr, "Unable to load options, rc %d\n", rc);
+}
options_free(&options);
}
+
+#define TEST(exp, ...) do { \
+char *argv[] = { "PROGRAM", __
[Spice-devel] [PATCH v2 fixup1 x11spice 2/3] Simplify the expression of argument parsing.
This fixes a bug with --config=handling.
Signed-off-by: Jeremy White
---
v2: Simplify even further.
---
src/main.c| 8 +
src/options.c | 96 ---
src/options.h | 5 +--
3 files changed, 47 insertions(+), 62 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..cecadc8d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,13 +67,7 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
-rc = options_parse_arguments(argc, argv, &session.options);
-if (rc)
-goto exit;
-
-rc = options_process_io(&session.options);
+rc = options_load(&session.options, argc, argv);
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index cff4ac17..3a59d964 100644
--- a/src/options.c
+++ b/src/options.c
@@ -51,11 +51,6 @@ static void str_replace(char **dest, const char *src)
*dest = src ? g_strdup(src) : NULL;
}
-void options_init(options_t *options)
-{
-memset(options, 0, sizeof(*options));
-}
-
static void ssl_options_free(ssl_options_t *ssl)
{
str_replace(&ssl->ca_cert_file, NULL);
@@ -66,24 +61,6 @@ static void ssl_options_free(ssl_options_t *ssl)
str_replace(&ssl->ciphersuite, NULL);
}
-void options_free(options_t *options)
-{
-str_replace(&options->display, NULL);
-str_replace(&options->listen, NULL);
-
-ssl_options_free(&options->ssl);
-str_replace(&options->spice_password, NULL);
-str_replace(&options->password_file, NULL);
-
-str_replace(&options->virtio_path, NULL);
-str_replace(&options->uinput_path, NULL);
-str_replace(&options->on_connect, NULL);
-str_replace(&options->on_disconnect, NULL);
-str_replace(&options->user_config_file, NULL);
-str_replace(&options->system_config_file, NULL);
-}
-
-
static void string_option(gchar **dest, GKeyFile *u, GKeyFile *s, const gchar
*section, const gchar *key)
{
gchar *ret = NULL;
@@ -160,7 +137,7 @@ static void usage(char *argv0)
printf("%s [--minimize]\n", indent);
}
-int options_handle_ssl(options_t *options, const char *spec)
+static int options_handle_ssl(options_t *options, const char *spec)
{
char *save = NULL;
char *in = g_strdup(spec);
@@ -202,7 +179,7 @@ int options_handle_ssl(options_t *options, const char *spec)
return rc;
}
-void options_handle_ssl_file_options(options_t *options,
+static void options_handle_ssl_file_options(options_t *options,
GKeyFile *userkey, GKeyFile *systemkey)
{
options->ssl.enabled = bool_option(userkey, systemkey, "ssl", "enabled");
@@ -214,17 +191,7 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
-int options_parse_arguments(int argc, char *argv[], options_t *options)
+static int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
int longindex = 0;
@@ -254,6 +221,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -348,7 +316,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
return rc;
}
-void options_from_config(options_t *options)
+static void options_from_config(options_t *options)
{
GKeyFile *userkey = g_key_file_new();
GKeyFile *systemkey = NULL;
@@ -467,7 +435,7 @@ static int generate_password(options_t *options)
return 0;
}
-int options_process_io(options_t *options)
+static int options_process_io(options_t *options)
{
int rc;
if (options->password_file) {
@@ -487,6 +455,45 @@ int options_process_io(options_t *options)
return 0;
}
+void options_init(options_t *options)
+{
+memset(options, 0, sizeof(*options));
+}
+
+void options_free(options_t *options)
+{
+str_replace(&options->display, NULL);
+str_replace(&options->listen, NULL);
+
+ssl_options_free(&options->ssl);
+str_replace(&options->spice_password, NULL);
+str_replace(&a
[Spice-devel] [PATCH v3 fixup1 x11spice 2/3] Simplify the expression of argument parsing.
This fixes a bug with --config=handling.
Removed old testing code as obsolete instead of updating unused code.
Signed-off-by: Jeremy White
---
v2: Simplify even further.
v3: Add a commit message about the removal of unused code, do not group
static and public functions
---
src/main.c| 8 +---
src/options.c | 51 +++
src/options.h | 5 +
3 files changed, 25 insertions(+), 39 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..cecadc8d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,13 +67,7 @@ int main(int argc, char *argv[])
** Parse arguments
**--*/
options_init(&session.options);
-options_handle_user_config(argc, argv, &session.options);
-options_from_config(&session.options);
-rc = options_parse_arguments(argc, argv, &session.options);
-if (rc)
-goto exit;
-
-rc = options_process_io(&session.options);
+rc = options_load(&session.options, argc, argv);
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index cff4ac17..f57a902f 100644
--- a/src/options.c
+++ b/src/options.c
@@ -160,7 +160,7 @@ static void usage(char *argv0)
printf("%s [--minimize]\n", indent);
}
-int options_handle_ssl(options_t *options, const char *spec)
+static int options_handle_ssl(options_t *options, const char *spec)
{
char *save = NULL;
char *in = g_strdup(spec);
@@ -202,7 +202,7 @@ int options_handle_ssl(options_t *options, const char *spec)
return rc;
}
-void options_handle_ssl_file_options(options_t *options,
+static void options_handle_ssl_file_options(options_t *options,
GKeyFile *userkey, GKeyFile *systemkey)
{
options->ssl.enabled = bool_option(userkey, systemkey, "ssl", "enabled");
@@ -214,17 +214,7 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl",
"ciphersuite");
}
-void options_handle_user_config(int argc, char *argv[], options_t *options)
-{
-int i;
-for (i = 1; i < argc - 1; i++)
-if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") ==
0) {
-options->user_config_file = g_strdup(argv[i + 1]);
-i++;
-}
-}
-
-int options_parse_arguments(int argc, char *argv[], options_t *options)
+static int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
int longindex = 0;
@@ -254,6 +244,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
{0, 0, 0, 0}
};
+optind = 1; /* Allow reuse of this function */
while (1) {
rc = getopt_long_only(argc, argv, "", long_options, &longindex);
if (rc == -1) {
@@ -348,7 +339,7 @@ int options_parse_arguments(int argc, char *argv[],
options_t *options)
return rc;
}
-void options_from_config(options_t *options)
+static void options_from_config(options_t *options)
{
GKeyFile *userkey = g_key_file_new();
GKeyFile *systemkey = NULL;
@@ -467,7 +458,7 @@ static int generate_password(options_t *options)
return 0;
}
-int options_process_io(options_t *options)
+static int options_process_io(options_t *options)
{
int rc;
if (options->password_file) {
@@ -487,6 +478,23 @@ int options_process_io(options_t *options)
return 0;
}
+int options_load(options_t *options, int argc, char *argv[])
+{
+int rc;
+
+rc = options_parse_arguments(argc, argv, options);
+if (rc == 0) {
+options_from_config(options);
+/* We parse command line arguments a second time to ensure
+** that command line options take precedence over config files */
+rc = options_parse_arguments(argc, argv, options);
+if (rc == 0)
+rc = options_process_io(options);
+}
+return rc;
+}
+
+
int options_impossible_config(options_t *options)
{
if (options->spice_password)
@@ -500,16 +508,3 @@ int options_impossible_config(options_t *options)
return 1;
}
-
-#if defined(OPTIONS_MAIN)
-int main(int argc, char *argv[])
-{
-options_t options;
-
-options_init(&options);
-options_parse_arguments(argc, argv, &options);
-options_from_config(&options);
-g_message("Options parsed");
-options_free(&options);
-}
-#endif
diff --git a/src/options.h b/src/options.h
index 6155984b..e7cdece1 100644
--- a/src/options.h
+++ b/src/options.h
@@ -73,11 +73,8 @@ typedef struct {
** Prototypes
**--*/
void options_init(options_t *options);
-void options_handle_user_config(int argc, char *argv[], options_t *options);
-int options_parse_arguments(int argc,
Re: [Spice-devel] [PATCH 1/2] Removed only written system_config_file field
On 7/26/19 9:28 AM, Frediano Ziglio wrote:
Signed-off-by: Frediano Ziglio
For series:
Acked-by: Jeremy White
---
src/options.c | 3 +--
src/options.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/options.c b/src/options.c
index c7c75a8..a6c7b9e 100644
--- a/src/options.c
+++ b/src/options.c
@@ -80,7 +80,6 @@ void options_free(options_t *options)
str_replace(&options->on_connect, NULL);
str_replace(&options->on_disconnect, NULL);
str_replace(&options->user_config_file, NULL);
-str_replace(&options->system_config_file, NULL);
}
@@ -353,7 +352,7 @@ static void options_from_config(options_t *options)
systemkey = g_key_file_new();
if (!g_key_file_load_from_dirs(systemkey, "x11spice/x11spice.conf",
(const char **)
g_get_system_config_dirs(),
- &options->system_config_file,
G_KEY_FILE_NONE, NULL)) {
+ NULL, G_KEY_FILE_NONE, NULL)) {
g_key_file_free(systemkey);
systemkey = NULL;
}
diff --git a/src/options.h b/src/options.h
index e7cdece..94341e8 100644
--- a/src/options.h
+++ b/src/options.h
@@ -65,7 +65,6 @@ typedef struct {
/* file names of config files */
char *user_config_file;
-char *system_config_file;
} options_t;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice v2 3/3] Add cache for SHM segments
Hi Brendan,
I've been running with this code for a while, and it works nicely. It's
hard to quantify, but with my test case I get cache hit rates in the 90%
range, and it does seem to improve overall performance.
I've got two comments on the patch, inline below:
On 7/16/19 9:16 PM, Brendan Shanks wrote:
Add a cache to allow the reuse of SHM segments.
Shared memory segments are added to the cache instead of being
deallocated, and the cache is searched instead of/before allocating a
new segment.
Both the SHM segments and their attachment with the X server are cached.
The cache currently has a fixed number of 10 entries, this provided a
good cache hit rate while keeping memory usage under control.
Building with DEBUG_SHM_CACHE defined and running with
G_MESSAGES_DEBUG=all will periodically print out the SHM cache hit
rate.
On my Ubuntu 18.04 system running XFCE4 with a 2560x1440 screen, the
cache hit rate starts around 72%. On-screen windows that update often
and have consistently-sized damage rectangles are the best case. With
several of those (scrolling terminal windows, web browser showing a
WebGL demo), the hit rate slowly rises to around 92%.
Operations that generate rapid damage reports (like resizing or moving
windows) will lower the hit rate.
Signed-off-by: Brendan Shanks
---
src/display.c | 177 ++
src/display.h | 7 +-
2 files changed, 169 insertions(+), 15 deletions(-)
diff --git a/src/display.c b/src/display.c
index 94f192c..1668d8a 100644
--- a/src/display.c
+++ b/src/display.c
@@ -213,11 +213,157 @@ static int register_for_events(display_t *d)
return 0;
}
+static void shm_segment_destroy(display_t *d, shm_segment_t *segment)
+{
+if (segment->shmid == -1) {
+return;
+}
+
+xcb_shm_detach(d->c, segment->shmseg);
+segment->shmseg = -1;
+
+shmdt(segment->shmaddr);
+segment->shmaddr = NULL;
+
+shmctl(segment->shmid, IPC_RMID, NULL);
+segment->shmid = -1;
+}
+
+
+static int shm_cache_get(display_t *d, size_t size, shm_segment_t *segment)
+{
+int i, ret;
+shm_segment_t *bigger_entry = NULL;
+shm_segment_t *entry_to_use = NULL;
+
+#if defined(DEBUG_SHM_CACHE)
+static guint cache_hits = 0;
+static guint cache_total = 0;
+
+cache_total++;
+#endif
+
+g_mutex_lock(&d->shm_cache_mutex);
+
+/* Check the cache for a segment of size 'size' or bigger.
+ * Use an exact-size segment if found.
+ * If not, use the smallest entry that is big enough.
+ */
+for (i = 0; i < G_N_ELEMENTS(d->shm_cache); i++) {
+shm_segment_t *entry = &d->shm_cache[i];
+
+if (entry->shmid != -1) {
+/* If a cache entry of the exact size being requested is found,
use that */
+if (size == entry->size) {
+entry_to_use = entry;
+break;
+}
+
+/* Keep track of the next-biggest entry in case an exact-size
match isn't found */
+if (size < entry->size) {
+if (bigger_entry && entry->size < bigger_entry->size) {
+bigger_entry = entry;
+} else {
+bigger_entry = entry;
+}
Isn't this code flawed? The else behavior should not be identical to
the if condition. Shouldn't it be something more like:
if(bigger_entry == NULL || entry->size < bigger_entry->size) {
bigger_entry = entry;
}
+}
+}
+}
+
+/* An exact-size entry wasn't found, use the next biggest entry that was
found */
+if (!entry_to_use) {
+entry_to_use = bigger_entry;
+}
+
+if (entry_to_use) {
+*segment = *entry_to_use;
+entry_to_use->shmid = -1;
+
+ret = 1;
+
+#if defined(DEBUG_SHM_CACHE)
+cache_hits++;
+if ((cache_hits % 100 == 0)) {
+g_debug("SHM cache hitrate: %u/%u (%.2f%%)", cache_hits,
cache_total,
+(float) ((float) cache_hits / (float) cache_total) * 100);
+}
+#endif
+} else {
+/* No usable entry found in the cache */
+ret = 0;
+}
+
+g_mutex_unlock(&d->shm_cache_mutex);
+return ret;
+}
+
+static int shm_cache_add(display_t *d, shm_segment_t *segment)
+{
+int i, ret;
+shm_segment_t *smallest_entry = NULL;
+shm_segment_t *entry_to_use = NULL;
+
+g_mutex_lock(&d->shm_cache_mutex);
+
+/* 'segment' is now unused, try to add it to the cache.
+ * Use an empty slot in the cache if available.
+ * If not, evict the smallest entry (which also must be smaller than
'segment') from the cache.
+ */
+for (i = 0; i < G_N_ELEMENTS(d->shm_cache); i++) {
+shm_segment_t *entry = &d->shm_cache[i];
+
+if (entry->shmid == -1) {
+/* Use an empty slot if found */
+entry_to_use = entry;
+break;
+}
+
+/* Keep track of the smallest entry that's smaller than 'segment' */
+if
[Spice-devel] [PATCH x11spice 2/2] Including missing header for inlined clock functions.
Fixes compilation on RHEL 7.3. Signed-off-by: Jeremy White --- spice-video-dummy/src/dummy.h | 1 + 1 file changed, 1 insertion(+) diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h index af77c177..cb8afc37 100644 --- a/spice-video-dummy/src/dummy.h +++ b/spice-video-dummy/src/dummy.h @@ -10,6 +10,7 @@ #include #endif #include +#include #define GLAMOR_FOR_XORG 1 #include "glamor.h" -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 1/2] Provide compatibility for Glamor in Xorg 1.17.
Signed-off-by: Jeremy White
---
spice-video-dummy/src/dummy.h | 4 ++--
spice-video-dummy/src/spicedummy_driver.c | 11 ++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index a2502902..af77c177 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -11,11 +11,11 @@
#endif
#include
-#include "compat-api.h"
-
#define GLAMOR_FOR_XORG 1
#include "glamor.h"
+#include "compat-api.h"
+
/* Supported chipsets */
typedef enum {
DUMMY_CHIP
diff --git a/spice-video-dummy/src/spicedummy_driver.c
b/spice-video-dummy/src/spicedummy_driver.c
index fc355f85..415f07dc 100644
--- a/spice-video-dummy/src/spicedummy_driver.c
+++ b/spice-video-dummy/src/spicedummy_driver.c
@@ -558,6 +558,15 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
int ret;
VisualPtr visual;
void *pixels;
+int glamor_flags = GLAMOR_USE_EGL_SCREEN;
+
+/* With Xorg prior to 1.18, we want more more flags in glamor_init */
+#if defined(GLAMOR_USE_SCREEN)
+glamor_flags |= GLAMOR_USE_SCREEN;
+#endif
+#if defined(GLAMOR_USE_PICTURE_SCREEN)
+glamor_flags |= GLAMOR_USE_PICTURE_SCREEN;
+#endif
/*
* we need to get the ScrnInfoRec for this screen, so let's allocate
@@ -618,7 +627,7 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
/* must be after RGB ordering fixed */
fbPictureInit(pScreen, 0, 0);
-if (dPtr->glamor && !glamor_init(pScreen, GLAMOR_USE_EGL_SCREEN)) {
+if (dPtr->glamor && !glamor_init(pScreen, glamor_flags)) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
"Failed to initialise glamor at ScreenInit() time.\n");
return FALSE;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 x11spice 2/2] Provide compatibility for Glamor in Xorg 1.17.
In Xorg 1.18, X changed so that GLAMOR_USE_EGL_SCREEN was the only
flag required and it implies the behavior previously requested
with the GLAMOR_USE_SCREEN and GLAMORE_USE_PICTURE_SCREEN flags.
Thus, if we are building against an older Xorg, we need to specify
those now deprecated flags.
Additionally, the compat-api header conflicts with the older
glamor header file, so it needs to be moved to be included
after glamor.h.
Signed-off-by: Jeremy White
---
v2: Provide more explanation
---
spice-video-dummy/src/dummy.h | 4 ++--
spice-video-dummy/src/spicedummy_driver.c | 15 ++-
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index a2502902..af77c177 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -11,11 +11,11 @@
#endif
#include
-#include "compat-api.h"
-
#define GLAMOR_FOR_XORG 1
#include "glamor.h"
+#include "compat-api.h"
+
/* Supported chipsets */
typedef enum {
DUMMY_CHIP
diff --git a/spice-video-dummy/src/spicedummy_driver.c
b/spice-video-dummy/src/spicedummy_driver.c
index fc355f85..1dbe87b2 100644
--- a/spice-video-dummy/src/spicedummy_driver.c
+++ b/spice-video-dummy/src/spicedummy_driver.c
@@ -558,6 +558,19 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
int ret;
VisualPtr visual;
void *pixels;
+int glamor_flags = GLAMOR_USE_EGL_SCREEN;
+
+/* In Xorg 1.18, X changed so that GLAMOR_USE_EGL_SCREEN was the only
+ flag required and it implies the behavior previously requested
+ with the GLAMOR_USE_SCREEN and GLAMORE_USE_PICTURE_SCREEN flags.
+ Thus, if we are building against an older Xorg, we need to specify
+ those now deprecated flags. */
+#if defined(GLAMOR_USE_SCREEN)
+glamor_flags |= GLAMOR_USE_SCREEN;
+#endif
+#if defined(GLAMOR_USE_PICTURE_SCREEN)
+glamor_flags |= GLAMOR_USE_PICTURE_SCREEN;
+#endif
/*
* we need to get the ScrnInfoRec for this screen, so let's allocate
@@ -618,7 +631,7 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
/* must be after RGB ordering fixed */
fbPictureInit(pScreen, 0, 0);
-if (dPtr->glamor && !glamor_init(pScreen, GLAMOR_USE_EGL_SCREEN)) {
+if (dPtr->glamor && !glamor_init(pScreen, glamor_flags)) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
"Failed to initialise glamor at ScreenInit() time.\n");
return FALSE;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5] Adjust the presentation of two byte scan codes.
The previous implementation worked strictly due to a bug which would
luckily generate roughly the right scan codes, although we would send
more codes than required.
For example, the old implementation would send 0xdf48e0 for 'up key down'
and '0xdfc8e0' for 'up key up'. Essentially, it stored the bytes
in reverse order and had a bug while flipping them.
This code stores them in the order we transmit them which simplifies
the code.
Signed-off-by: Jeremy White
---
src/utils.js | 40 ++--
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/src/utils.js b/src/utils.js
index 1874e97..af8e574 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -157,6 +157,7 @@ common_scanmap[16] = KeyNames.KEY_ShiftL;
common_scanmap[17] = KeyNames.KEY_LCtrl;
common_scanmap[18] = KeyNames.KEY_Alt;
common_scanmap[20] = KeyNames.KEY_CapsLock;
+common_scanmap[44] = KeyNames.KEY_SysReqest;
common_scanmap[144]= KeyNames.KEY_NumLock;
common_scanmap[112]= KeyNames.KEY_F1;
common_scanmap[113]= KeyNames.KEY_F2;
@@ -174,19 +175,18 @@ common_scanmap[123]= KeyNames.KEY_F12;
/* These extended scancodes do not line up with values from atKeynames */
common_scanmap[42] = 99;
common_scanmap[19] = 101;// Break
-common_scanmap[111]= 0xE035; // KP_Divide
-common_scanmap[106]= 0xE037; // KP_Multiply
-common_scanmap[36] = 0xE047; // Home
-common_scanmap[38] = 0xE048; // Up
-common_scanmap[33] = 0xE049; // PgUp
-common_scanmap[37] = 0xE04B; // Left
-common_scanmap[39] = 0xE04D; // Right
-common_scanmap[35] = 0xE04F; // End
-common_scanmap[40] = 0xE050; // Down
-common_scanmap[34] = 0xE051; // PgDown
-common_scanmap[45] = 0xE052; // Insert
-common_scanmap[46] = 0xE053; // Delete
-common_scanmap[44] = 0x2A37; // Print
+common_scanmap[111]= 0xE0 | (KeyNames.KEY_Slash << 8);//
KP_Divide
+common_scanmap[106]= 0xE0 | (KeyNames.KEY_KP_Multiply << 8);
// KP_Multiply
+common_scanmap[36] = 0xE0 | (KeyNames.KEY_KP_7 << 8); // Home
+common_scanmap[38] = 0xE0 | (KeyNames.KEY_KP_8 << 8); // Up
+common_scanmap[33] = 0xE0 | (KeyNames.KEY_KP_9 << 8); // PgUp
+common_scanmap[37] = 0xE0 | (KeyNames.KEY_KP_4 << 8); // Left
+common_scanmap[39] = 0xE0 | (KeyNames.KEY_KP_6 << 8); // Right
+common_scanmap[35] = 0xE0 | (KeyNames.KEY_KP_1 << 8); // End
+common_scanmap[40] = 0xE0 | (KeyNames.KEY_KP_2 << 8); // Down
+common_scanmap[34] = 0xE0 | (KeyNames.KEY_KP_3 << 8); // PgDown
+common_scanmap[45] = 0xE0 | (KeyNames.KEY_KP_0 << 8); // Insert
+common_scanmap[46] = 0xE0 | (KeyNames.KEY_KP_Decimal << 8); //
Delete
/* These are not common between ALL browsers but are between Firefox and DOM3
*/
common_scanmap['1'.charCodeAt(0)] = KeyNames.KEY_1;
@@ -221,9 +221,9 @@ common_scanmap[222]= KeyNames.KEY_Quote;
common_scanmap[219]= KeyNames.KEY_LBrace;
common_scanmap[221]= KeyNames.KEY_RBrace;
-common_scanmap[91] = 0xE05B; //KeyNames.KEY_LMeta
-common_scanmap[92] = 0xE05C; //KeyNames.KEY_RMeta
-common_scanmap[93] = 0xE05D; //KeyNames.KEY_Menu
+common_scanmap[91] = 0xE0 | (0x5B << 8); //KeyNames.KEY_LMeta
+common_scanmap[92] = 0xE0 | (0x5C << 8); //KeyNames.KEY_RMeta
+common_scanmap[93] = 0xE0 | (0x5D << 8); //KeyNames.KEY_Menu
/* Firefox/Mozilla codes */
var firefox_scanmap = [];
@@ -260,11 +260,7 @@ function keycode_to_start_scan(code)
return 0;
}
-if (scancode < 0x100) {
-return scancode;
-} else {
-return 0xe0 | ((scancode - 0x100) << 8);
-}
+return scancode;
}
function keycode_to_end_scan(code)
@@ -276,7 +272,7 @@ function keycode_to_end_scan(code)
if (scancode < 0x100) {
return scancode | 0x80;
} else {
-return 0x80e0 | ((scancode - 0x100) << 8);
+return scancode | 0x8000;
}
}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 4/4] Adjust the presentation of two byte scan codes.
The previous implementation worked strictly due to a bug which would
luckily generate roughly the right scan codes, although we would send
more codes than required.
For example, the old implementation would send 0xdf48e0 for 'up key down'
and '0xdfc8e0' for 'up key up'. The prepended 0xdf is incorrect; the
correct values should be 0x48e0 and 0xc8e0. Essentially, it stored
the bytes in reverse order and had a bug while flipping them.
This code stores them in the order we transmit them which simplifies
the code.
Signed-off-by: Jeremy White
---
src/utils.js | 38 +-
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/utils.js b/src/utils.js
index 79e3e5e..1013789 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -223,21 +223,21 @@ common_scanmap[19] = 101;// Break, XSpice
only
were grey on the original AT keyboard. These are
prefixed, as they were on the PS/2 controller, with an
0xE0 byte to indicate that they are extended */
-common_scanmap[111]= 0xE035; // KP_Divide
-common_scanmap[106]= 0xE037; // KP_Multiply
-common_scanmap[36] = 0xE047; // Home
-common_scanmap[38] = 0xE048; // Up
-common_scanmap[33] = 0xE049; // PgUp
-common_scanmap[37] = 0xE04B; // Left
-common_scanmap[39] = 0xE04D; // Right
-common_scanmap[35] = 0xE04F; // End
-common_scanmap[40] = 0xE050; // Down
-common_scanmap[34] = 0xE051; // PgDown
-common_scanmap[45] = 0xE052; // Insert
-common_scanmap[46] = 0xE053; // Delete
-common_scanmap[91] = 0xE05B; //KeyNames.KEY_LMeta
-common_scanmap[92] = 0xE05C; //KeyNames.KEY_RMeta
-common_scanmap[93] = 0xE05D; //KeyNames.KEY_Menu
+common_scanmap[111]= 0xE0 | (KeyNames.KEY_Slash << 8);//
KP_Divide
+common_scanmap[106]= 0xE0 | (KeyNames.KEY_KP_Multiply << 8);
// KP_Multiply
+common_scanmap[36] = 0xE0 | (KeyNames.KEY_KP_7 << 8); // Home
+common_scanmap[38] = 0xE0 | (KeyNames.KEY_KP_8 << 8); // Up
+common_scanmap[33] = 0xE0 | (KeyNames.KEY_KP_9 << 8); // PgUp
+common_scanmap[37] = 0xE0 | (KeyNames.KEY_KP_4 << 8); // Left
+common_scanmap[39] = 0xE0 | (KeyNames.KEY_KP_6 << 8); // Right
+common_scanmap[35] = 0xE0 | (KeyNames.KEY_KP_1 << 8); // End
+common_scanmap[40] = 0xE0 | (KeyNames.KEY_KP_2 << 8); // Down
+common_scanmap[34] = 0xE0 | (KeyNames.KEY_KP_3 << 8); // PgDown
+common_scanmap[45] = 0xE0 | (KeyNames.KEY_KP_0 << 8); // Insert
+common_scanmap[46] = 0xE0 | (KeyNames.KEY_KP_Decimal << 8); //
Delete
+common_scanmap[91] = 0xE0 | (0x5B << 8); //KeyNames.KEY_LMeta
+common_scanmap[92] = 0xE0 | (0x5C << 8); //KeyNames.KEY_RMeta
+common_scanmap[93] = 0xE0 | (0x5D << 8); //KeyNames.KEY_Menu
/* Firefox/Mozilla codes */
var firefox_scanmap = [];
@@ -273,11 +273,7 @@ function keycode_to_start_scan(code)
return 0;
}
-if (scancode < 0x100) {
-return scancode;
-} else {
-return 0xe0 | ((scancode - 0x100) << 8);
-}
+return scancode;
}
function keycode_to_end_scan(code)
@@ -289,7 +285,7 @@ function keycode_to_end_scan(code)
if (scancode < 0x100) {
return scancode | 0x80;
} else {
-return 0x80e0 | ((scancode - 0x100) << 8);
+return scancode | 0x8000;
}
}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 1/4] Use a named constant from atKeynames.js for the PrintScreen/SysRq key.
Correct a typo from the upstream atKeynames.js at the same time.
Signed-off-by: Jeremy White
---
src/atKeynames.js | 2 +-
src/utils.js | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/atKeynames.js b/src/atKeynames.js
index 110e9e9..767e78b 100644
--- a/src/atKeynames.js
+++ b/src/atKeynames.js
@@ -174,7 +174,7 @@ var KeyNames = {
KEY_KP_3:/* 3 PgDown0x51 */ 81,
KEY_KP_0:/* 0 Insert0x52 */ 82,
KEY_KP_Decimal :/* . (Decimal) Delete0x53 */ 83,
- KEY_SysReqest :/* SysReqest 0x54 */ 84,
+ KEY_SysRequest :/* SysRequest0x54 */ 84,
/* NOTUSED 0x55 */
KEY_Less:/* < (Less) >(Greater) 0x56 */ 86,
KEY_F11 :/* F11 0x57 */ 87,
diff --git a/src/utils.js b/src/utils.js
index 1874e97..f12edf9 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -157,6 +157,7 @@ common_scanmap[16] = KeyNames.KEY_ShiftL;
common_scanmap[17] = KeyNames.KEY_LCtrl;
common_scanmap[18] = KeyNames.KEY_Alt;
common_scanmap[20] = KeyNames.KEY_CapsLock;
+common_scanmap[44] = KeyNames.KEY_SysReqest;
common_scanmap[144]= KeyNames.KEY_NumLock;
common_scanmap[112]= KeyNames.KEY_F1;
common_scanmap[113]= KeyNames.KEY_F2;
@@ -186,7 +187,6 @@ common_scanmap[40] = 0xE050; // Down
common_scanmap[34] = 0xE051; // PgDown
common_scanmap[45] = 0xE052; // Insert
common_scanmap[46] = 0xE053; // Delete
-common_scanmap[44] = 0x2A37; // Print
/* These are not common between ALL browsers but are between Firefox and DOM3
*/
common_scanmap['1'.charCodeAt(0)] = KeyNames.KEY_1;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 3/4] Update the documentation and organization of the scancodes.
The common scan codes were disjointed and logically belong together.
Signed-off-by: Jeremy White
---
src/utils.js | 101 +--
1 file changed, 57 insertions(+), 44 deletions(-)
diff --git a/src/utils.js b/src/utils.js
index 5059a5b..79e3e5e 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -114,14 +114,54 @@ function arraybuffer_to_str(buf) {
}
/*
-** Converting keycodes to AT scancodes is very hard.
-** luckly there are some resources on the web and in the Xorg driver that help
-** us figure out what browser dependent keycodes match to what scancodes.
-**
-** This will most likely not work for non US keyboard and browsers other than
-** modern Chrome and FireFox.
+** Converting browser keycodes to AT scancodes is very hard.
+** Spice transmits keys using the original AT scan codes, often
+** described as 'Scan Code Set 1'.
+** There is a confusion of other scan codes; Xorg synthesizes it's
+** own in the same atKeynames.c file that has the XT codes.
+** Scan code set 2 and 3 are more common, and use different values.
+** Further, there is no formal specification for keycodes
+** returned by browsers, so we have done our mapping largely with
+** empirical testing.
+** There has been little rigorous testing with International keyboards,
+** and this would be an easy area for non English speakers to contribute.
**--*/
var common_scanmap = [];
+
+/* The following appear to be keycodes that work in most browsers */
+common_scanmap['1'.charCodeAt(0)] = KeyNames.KEY_1;
+common_scanmap['2'.charCodeAt(0)] = KeyNames.KEY_2;
+common_scanmap['3'.charCodeAt(0)] = KeyNames.KEY_3;
+common_scanmap['4'.charCodeAt(0)] = KeyNames.KEY_4;
+common_scanmap['5'.charCodeAt(0)] = KeyNames.KEY_5;
+common_scanmap['6'.charCodeAt(0)] = KeyNames.KEY_6;
+common_scanmap['7'.charCodeAt(0)] = KeyNames.KEY_7;
+common_scanmap['8'.charCodeAt(0)] = KeyNames.KEY_8;
+common_scanmap['9'.charCodeAt(0)] = KeyNames.KEY_9;
+common_scanmap['0'.charCodeAt(0)] = KeyNames.KEY_0;
+common_scanmap[145]= KeyNames.KEY_ScrollLock;
+common_scanmap[103]= KeyNames.KEY_KP_7;
+common_scanmap[104]= KeyNames.KEY_KP_8;
+common_scanmap[105]= KeyNames.KEY_KP_9;
+common_scanmap[100]= KeyNames.KEY_KP_4;
+common_scanmap[101]= KeyNames.KEY_KP_5;
+common_scanmap[102]= KeyNames.KEY_KP_6;
+common_scanmap[107]= KeyNames.KEY_KP_Plus;
+common_scanmap[97] = KeyNames.KEY_KP_1;
+common_scanmap[98] = KeyNames.KEY_KP_2;
+common_scanmap[99] = KeyNames.KEY_KP_3;
+common_scanmap[96] = KeyNames.KEY_KP_0;
+common_scanmap[109]= KeyNames.KEY_Minus;
+common_scanmap[110]= KeyNames.KEY_KP_Decimal;
+common_scanmap[191]= KeyNames.KEY_Slash;
+common_scanmap[190]= KeyNames.KEY_Period;
+common_scanmap[188]= KeyNames.KEY_Comma;
+common_scanmap[220]= KeyNames.KEY_BSlash;
+common_scanmap[192]= KeyNames.KEY_Tilde;
+common_scanmap[222]= KeyNames.KEY_Quote;
+common_scanmap[219]= KeyNames.KEY_LBrace;
+common_scanmap[221]= KeyNames.KEY_RBrace;
+
common_scanmap['Q'.charCodeAt(0)] = KeyNames.KEY_Q;
common_scanmap['W'.charCodeAt(0)] = KeyNames.KEY_W;
common_scanmap['E'.charCodeAt(0)] = KeyNames.KEY_E;
@@ -172,9 +212,17 @@ common_scanmap[121]= KeyNames.KEY_F10;
common_scanmap[122]= KeyNames.KEY_F11;
common_scanmap[123]= KeyNames.KEY_F12;
-/* These extended scancodes do not line up with values from atKeynames */
-common_scanmap[42] = 99;
-common_scanmap[19] = 101;// Break
+/* TODO: Break and Print are complex scan codes. XSpice cheats and
+ uses Xorg synthesized codes to simplify them. Fixing this will
+ require XSpice to handle the scan codes correctly, and then
+ fix spice-html5 to send the complex scan codes. */
+common_scanmap[42] = 99; // Print, XSpice only
+common_scanmap[19] = 101;// Break, XSpice only
+
+/* Handle the so called 'GREY' keys, for the extended keys that
+ were grey on the original AT keyboard. These are
+ prefixed, as they were on the PS/2 controller, with an
+ 0xE0 byte to indicate that they are extended */
common_scanmap[111]= 0xE035; // KP_Divide
common_scanmap[106]= 0xE037; // KP_Multiply
common_scanmap[36] = 0xE047; // Home
@@ -187,41 +235,6 @@ common_scanmap[40]
[Spice-devel] [PATCH spice-html5 2/4] Support the keypad minus key in Chrome.
The keypad minus key at 109 is common between at least Chrome and Firefox. Signed-off-by: Jeremy White --- src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.js b/src/utils.js index f12edf9..5059a5b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -211,6 +211,7 @@ common_scanmap[97] = KeyNames.KEY_KP_1; common_scanmap[98] = KeyNames.KEY_KP_2; common_scanmap[99] = KeyNames.KEY_KP_3; common_scanmap[96] = KeyNames.KEY_KP_0; +common_scanmap[109]= KeyNames.KEY_Minus; common_scanmap[110]= KeyNames.KEY_KP_Decimal; common_scanmap[191]= KeyNames.KEY_Slash; common_scanmap[190]= KeyNames.KEY_Period; @@ -228,7 +229,6 @@ common_scanmap[93] = 0xE05D; //KeyNames.KEY_Menu /* Firefox/Mozilla codes */ var firefox_scanmap = []; firefox_scanmap[173]= KeyNames.KEY_Minus; -firefox_scanmap[109]= KeyNames.KEY_Minus; firefox_scanmap[61] = KeyNames.KEY_Equal; firefox_scanmap[59] = KeyNames.KEY_SemiColon; -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-html5 0/4] Improve spice-html5 keyboard code
This is effectively v2 of 'Adjust the presentation of two byte scan codes.' It breaks out a part of the patch separately. In reviewing this, I caught a number of other issues. One warranted a patch (included). For the rest, I chose to 'fix' it by updating the documentation, which was a bit out of date. The original patch has it's comment tweaked. Frediano had a specific request to not do the (<< 8) mechanic on the symbols; I have not changed that, as I prefer that expression. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice] Fix running tests on Debian/Ubuntu
On 8/5/19 1:19 PM, Brendan Shanks wrote:
Check the Debian/Ubuntu path for the non-wrapper Xorg binary
Signed-off-by: Brendan Shanks
Acked-by: Jeremy White
---
src/tests/xdummy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/tests/xdummy.c b/src/tests/xdummy.c
index 0651dc7..ce57b1c 100644
--- a/src/tests/xdummy.c
+++ b/src/tests/xdummy.c
@@ -157,6 +157,8 @@ static int exec_xorg(xdummy_t *server, gconstpointer
user_data G_GNUC_UNUSED)
strcpy(xorg_binary, "Xorg");
if (access("/usr/libexec/Xorg", X_OK) == 0)
strcpy(xorg_binary, "/usr/libexec/Xorg");
+else if (access("/usr/lib/xorg/Xorg", X_OK) == 0)
+strcpy(xorg_binary, "/usr/lib/xorg/Xorg");
return execlp(xorg_binary, xorg_binary, "-ac",
"-config", server->xorg_fname,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice] Use separate buffer for primary surface to fix graphical corruption
Hi Brendan,
Nicely done; this solves the problem I had with repeated test runs.
I have one trivial change to request:
On 8/5/19 2:12 PM, Brendan Shanks wrote:
The 'display->fullscreen' SHM segment was previously being used for both
x11spice's internal change scanning and as the spice primary surface.
I don't think spice wants anything else writing to the primary surface,
and this caused sporadic test failures and graphical corruption.
Create a separate SHM segment 'display->primary' and use it only for the
primary surface.
Signed-off-by: Brendan Shanks
---
src/display.c | 20 +++-
src/display.h | 1 +
src/main.c| 2 +-
src/session.c | 2 +-
4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/display.c b/src/display.c
index 01e0e85..77b4d4e 100644
--- a/src/display.c
+++ b/src/display.c
@@ -461,12 +461,25 @@ void destroy_shm_image(display_t *d, shm_image_t *shmi)
int display_create_screen_images(display_t *d)
{
+/* 'primary' and 'fullscreen' don't need to be SHM, normal buffers would
work
+ * fine. Using SHM for all buffers is simpler though, and has no real
downsides.
+ */
+d->primary = create_shm_image(d, 0, 0);
+if (!d->primary) {
+return X11SPICE_ERR_NOSHM;
+}
+
d->fullscreen = create_shm_image(d, 0, 0);
-if (!d->fullscreen)
+if (!d->fullscreen) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
return X11SPICE_ERR_NOSHM;
+}
d->scanline = create_shm_image(d, 0, 1);
if (!d->scanline) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
return X11SPICE_ERR_NOSHM;
@@ -477,6 +490,11 @@ int display_create_screen_images(display_t *d)
void display_destroy_screen_images(display_t *d)
{
+if (d->primary) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
+}
+
if (d->fullscreen) {
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
diff --git a/src/display.h b/src/display.h
index dc4254b..27bc829 100644
--- a/src/display.h
+++ b/src/display.h
@@ -55,6 +55,7 @@ typedef struct {
const xcb_query_extension_reply_t *xfixes_ext;
+shm_image_t *primary;
shm_image_t *fullscreen;
shm_image_t *scanline;
diff --git a/src/main.c b/src/main.c
index cecadc8..71cbb46 100644
--- a/src/main.c
+++ b/src/main.c
@@ -108,7 +108,7 @@ int main(int argc, char *argv[])
/*
** Start up a spice server
**--*/
-rc = spice_start(&session.spice, &session.options,
session.display.fullscreen);
+rc = spice_start(&session.spice, &session.options,
session.display.primary);
Could you modify the local variables inside the spice_start function to
use 'primary' instead of 'fullscreen' so we are not at a risk of
conflating them in the future?
Cheers,
Jeremy
if (rc)
goto exit;
spice_started = 1;
diff --git a/src/session.c b/src/session.c
index 1e59415..4d72e14 100644
--- a/src/session.c
+++ b/src/session.c
@@ -352,7 +352,7 @@ int session_recreate_primary(session_t *s)
rc = display_create_screen_images(&s->display);
if (rc == 0) {
-shm_image_t *f = s->display.fullscreen;
+shm_image_t *f = s->display.primary;
rc = spice_create_primary(&s->spice, f->w, f->h, f->bytes_per_line,
f->shmaddr);
}
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice v2] Use separate buffer for primary surface to fix graphical corruption
On 8/6/19 11:02 AM, Brendan Shanks wrote:
The 'display->fullscreen' SHM segment was previously being used for both
x11spice's internal change scanning and as the spice primary surface.
I don't think spice wants anything else writing to the primary surface,
and this caused sporadic test failures and graphical corruption.
Create a separate SHM segment 'display->primary' and use it only for the
primary surface.
Signed-off-by: Brendan Shanks
Acked-by: Jeremy White
---
v2: Rename local variable 'fullscreen' to 'primary' in spice_start()
---
src/display.c | 20 +++-
src/display.h | 1 +
src/local_spice.h | 2 +-
src/main.c| 2 +-
src/session.c | 2 +-
src/spice.c | 6 +++---
6 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/display.c b/src/display.c
index 01e0e85..77b4d4e 100644
--- a/src/display.c
+++ b/src/display.c
@@ -461,12 +461,25 @@ void destroy_shm_image(display_t *d, shm_image_t *shmi)
int display_create_screen_images(display_t *d)
{
+/* 'primary' and 'fullscreen' don't need to be SHM, normal buffers would
work
+ * fine. Using SHM for all buffers is simpler though, and has no real
downsides.
+ */
+d->primary = create_shm_image(d, 0, 0);
+if (!d->primary) {
+return X11SPICE_ERR_NOSHM;
+}
+
d->fullscreen = create_shm_image(d, 0, 0);
-if (!d->fullscreen)
+if (!d->fullscreen) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
return X11SPICE_ERR_NOSHM;
+}
d->scanline = create_shm_image(d, 0, 1);
if (!d->scanline) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
return X11SPICE_ERR_NOSHM;
@@ -477,6 +490,11 @@ int display_create_screen_images(display_t *d)
void display_destroy_screen_images(display_t *d)
{
+if (d->primary) {
+destroy_shm_image(d, d->primary);
+d->primary = NULL;
+}
+
if (d->fullscreen) {
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
diff --git a/src/display.h b/src/display.h
index dc4254b..27bc829 100644
--- a/src/display.h
+++ b/src/display.h
@@ -55,6 +55,7 @@ typedef struct {
const xcb_query_extension_reply_t *xfixes_ext;
+shm_image_t *primary;
shm_image_t *fullscreen;
shm_image_t *scanline;
diff --git a/src/local_spice.h b/src/local_spice.h
index 9d5e989..0573500 100644
--- a/src/local_spice.h
+++ b/src/local_spice.h
@@ -61,7 +61,7 @@ typedef struct {
/*
** Prototypes
**--*/
-int spice_start(spice_t *s, options_t *options, shm_image_t *fullscreen);
+int spice_start(spice_t *s, options_t *options, shm_image_t *primary);
void spice_end(spice_t *s);
int spice_create_primary(spice_t *s, int w, int h, int bytes_per_line, void
*shmaddr);
void spice_destroy_primary(spice_t *s);
diff --git a/src/main.c b/src/main.c
index cecadc8..71cbb46 100644
--- a/src/main.c
+++ b/src/main.c
@@ -108,7 +108,7 @@ int main(int argc, char *argv[])
/*
** Start up a spice server
**--*/
-rc = spice_start(&session.spice, &session.options,
session.display.fullscreen);
+rc = spice_start(&session.spice, &session.options,
session.display.primary);
if (rc)
goto exit;
spice_started = 1;
diff --git a/src/session.c b/src/session.c
index 1e59415..4d72e14 100644
--- a/src/session.c
+++ b/src/session.c
@@ -352,7 +352,7 @@ int session_recreate_primary(session_t *s)
rc = display_create_screen_images(&s->display);
if (rc == 0) {
-shm_image_t *f = s->display.fullscreen;
+shm_image_t *f = s->display.primary;
rc = spice_create_primary(&s->spice, f->w, f->h, f->bytes_per_line,
f->shmaddr);
}
diff --git a/src/spice.c b/src/spice.c
index 430df40..cf7118f 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -633,7 +633,7 @@ static int try_listen(spice_t *s, options_t *options)
return 0;
}
-int spice_start(spice_t *s, options_t *options, shm_image_t *fullscreen)
+int spice_start(spice_t *s, options_t *options, shm_image_t *primary)
{
int rc;
@@ -678,8 +678,8 @@ int spice_start(spice_t *s, options_t *options, shm_image_t *fullscreen)
spice_server_vm_start(s->server);
-rc = spice_create_primary(s, fullscreen->w, fullscreen->h,
- fullsc
Re: [Spice-devel] [PATCH spice-html5] Review the webm audio track header and remove the fixmes.
On 8/11/19 4:32 AM, Frediano Ziglio wrote:
Hi,
why this was not merged ?
I completely missed the ack at the time.
I do think the patch is fundamentally correct and an improvement. This
one arguably deserves a reprise of the audit I performed at the time to
ensure that the results are still correct. I was planning to do that in
September (and resubmit the uid patch at the same time).
For the record, I have an internal todo list of patches that were missed
to circle back to. You've now caught most of them, thank you
(websockets being the big one). I believe I still have one or two
patches to argument parsing on libcacard, and I do not have a patch, but
a concern, on the removal of the spice-controller. I also have a large
separate patch set against the kernel providing usbredir support.
Cheers,
Jeremy
Frediano
Ack
Thanks,
Pavel
On Fri, 2016-12-23 at 10:53 -0600, Jeremy White wrote:
This involved a review of the Firefox parsing code along
with the official specifcation, and setting these fields
to the specified default values.
Most notably, I have found that recent browsers do not need the
8 ms pre skip, and it seems to remove some audio lag to
switch to 0.
Signed-off-by: Jeremy White
---
webm.js | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/webm.js b/webm.js
index 789da14..72c1853 100644
--- a/webm.js
+++ b/webm.js
@@ -393,28 +393,39 @@ webm_SeekHead.prototype =
function webm_AudioTrackEntry()
{
+/*
+** In general, we follow this specification:
+** https://www.matroska.org/technical/specs/index.html
+** we supply the mandatory values, and a comment notes
+** where we differ from the default
+** There is one RECOMMENDED guideline we are omitting;
+** the OPUS codec_delay is recommended to be 80ms.
+** However, the spice server does not currently provide time
+** stamps that are offset by 80ms, so you effectively get an
+** 80ms lag with this setting.
+*/
this.id = WEBM_TRACK_ENTRY;
this.number = 1;
this.uid = 1;
this.type = 2; // Audio
this.flag_enabled = 1;
this.flag_default = 1;
-this.flag_forced = 1;
-this.flag_lacing = 0;
-this.min_cache = 0; // fixme - check
+this.flag_forced = 1; // Different than default; we wish to
force
+this.flag_lacing = 1;
+this.min_cache = 0;
this.max_block_addition_id = 0;
-this.codec_decode_all = 0; // fixme - check
-this.seek_pre_roll = 0; // 8000; // fixme - check
-this.codec_delay = 8000; // Must match
codec_private.preskip
+this.seek_pre_roll = 0;
+this.codec_delay = 0; // Must match codec_private.preskip
this.codec_id = "A_OPUS";
+this.codec_decode_all = 1;
this.audio = new webm_Audio(OPUS_FREQUENCY);
// See: http://tools.ietf.org/html/draft-terriberry-oggopus-01
this.codec_private = [ 0x4f, 0x70, 0x75, 0x73, 0x48, 0x65,
0x61, 0x64, // OpusHead
0x01, // Version
OPUS_CHANNELS,
- 0x00, 0x0F, // Preskip - 3840 samples -
should be 8ms at 48kHz
- 0x80, 0xbb, 0x00, 0x00, // 48000
+ 0x00, 0x00, // Preskip - 3840 samples
would be 8ms at 48kHz
+ 0x80, 0xbb, 0x00, 0x00, // nominal rate
- 48000
0x00, 0x00, // Output gain
0x00 // Channel mapping family
];
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice v3 3/3] Add cache for SHM segments
Hi,
I concur with Frediano on those two changes, but this code is otherwise
working well for me.
Cheers,
Jeremy
On 8/17/19 12:05 AM, Frediano Ziglio wrote:
Add a cache to allow the reuse of SHM segments.
Shared memory segments are added to the cache instead of being
deallocated, and the cache is searched instead of/before allocating a
new segment.
Both the SHM segments and their attachment with the X server are cached.
The cache currently has a fixed number of 10 entries, this provided a
good cache hit rate while keeping memory usage under control.
Building with DEBUG_SHM_CACHE defined and running with
G_MESSAGES_DEBUG=all will periodically print out the SHM cache hit
rate.
On my Ubuntu 18.04 system running XFCE4 with a 2560x1440 screen, the
cache hit rate starts around 72%. On-screen windows that update often
and have consistently-sized damage rectangles are the best case. With
several of those (scrolling terminal windows, web browser showing a
WebGL demo), the hit rate slowly rises to around 92%.
Operations that generate rapid damage reports (like resizing or moving
windows) will lower the hit rate.
Signed-off-by: Brendan Shanks
---
src/display.c | 175 ++
src/display.h | 11 +++-
2 files changed, 170 insertions(+), 16 deletions(-)
diff --git a/src/display.c b/src/display.c
index a2a18b8..55461a2 100644
--- a/src/display.c
+++ b/src/display.c
@@ -213,11 +213,155 @@ static int register_for_events(display_t *d)
return 0;
}
+static void shm_segment_destroy(display_t *d, shm_segment_t *segment)
+{
+if (segment->shmid == -1) {
+return;
+}
+
+xcb_shm_detach(d->c, segment->shmseg);
+segment->shmseg = -1;
+
+shmdt(segment->shmaddr);
+segment->shmaddr = NULL;
+
+shmctl(segment->shmid, IPC_RMID, NULL);
+segment->shmid = -1;
+}
+
+
+static int shm_cache_get(display_t *d, size_t size, shm_segment_t *segment)
+{
+int i, ret;
+shm_segment_t *bigger_entry = NULL;
+shm_segment_t *entry_to_use = NULL;
+
+#if defined(DEBUG_SHM_CACHE)
+static guint cache_hits = 0;
+static guint cache_total = 0;
+
+cache_total++;
+#endif
+
+g_mutex_lock(&d->shm_cache_mutex);
+
+/* Check the cache for a segment of size 'size' or bigger.
+ * Use an exact-size segment if found.
+ * If not, use the smallest entry that is big enough.
+ */
+for (i = 0; i < G_N_ELEMENTS(d->shm_cache); i++) {
+shm_segment_t *entry = &d->shm_cache[i];
+
+if (entry->shmid != -1) {
+/* If a cache entry of the exact size being requested is found,
use that */
+if (size == entry->size) {
+entry_to_use = entry;
+break;
+}
+
+/* Keep track of the next-biggest entry in case an exact-size
match isn't found */
+if (size < entry->size) {
+if ((bigger_entry && entry->size < bigger_entry->size) ||
!bigger_entry) {
This is equivalent to
if (!bigger_entry || entry->size < bigger_entry->size) {
+bigger_entry = entry;
+}
+}
+}
+}
+
+/* An exact-size entry wasn't found, use the next biggest entry that was
found */
+if (!entry_to_use) {
+entry_to_use = bigger_entry;
+}
+
+if (entry_to_use) {
+*segment = *entry_to_use;
+entry_to_use->shmid = -1;
+
+ret = 1;
+
+#if defined(DEBUG_SHM_CACHE)
+cache_hits++;
+if ((cache_hits % 100 == 0)) {
+g_debug("SHM cache hitrate: %u/%u (%.2f%%)", cache_hits,
cache_total,
+(float) ((float) cache_hits / (float) cache_total) *
100);
+}
+#endif
+} else {
+/* No usable entry found in the cache */
+ret = 0;
+}
+
+g_mutex_unlock(&d->shm_cache_mutex);
+return ret;
+}
+
+static int shm_cache_add(display_t *d, shm_segment_t *segment)
+{
+int i, ret;
+shm_segment_t *smallest_entry = NULL;
+shm_segment_t *entry_to_use = NULL;
+
+g_mutex_lock(&d->shm_cache_mutex);
+
+/* 'segment' is now unused, try to add it to the cache.
+ * Use an empty slot in the cache if available.
+ * If not, evict the smallest entry (which also must be smaller than
'segment') from the cache.
+ */
+for (i = 0; i < G_N_ELEMENTS(d->shm_cache); i++) {
+shm_segment_t *entry = &d->shm_cache[i];
+
+if (entry->shmid == -1) {
+/* Use an empty slot if found */
+entry_to_use = entry;
+break;
+}
+
+/* Keep track of the smallest entry that's smaller than 'segment' */
+if (entry->size < segment->size) {
+if (smallest_entry && entry->size < smallest_entry->size) {
+smallest_entry = entry;
+} else if (!smallest_entry) {
Similar here (single if),
if (!smallest_entry || entry->size < smallest_entry->size) {
+smallest_entry = entry;
+}
+
Re: [Spice-devel] [PATCH x11spice v4 0/3] Add cache for SHM segments
Series: Acked-by: Jeremy White and pushed. Cheers, Jeremy On 8/19/19 8:03 PM, Brendan Shanks wrote: Add a cache to x11spice for SHM segments. v4 simplifies the 'if' clauses in shm_cache_get/add, as suggested by Frediano. Brendan Shanks (3): Use unsigned int/size_t for display width/height/buffer size Create separate shm_segment_t struct for SHM segments Add cache for SHM segments doc/spice_indent | 1 + src/display.c| 205 --- src/display.h| 29 --- src/scan.c | 2 +- src/session.c| 2 +- src/spice.c | 2 +- 6 files changed, 199 insertions(+), 42 deletions(-) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 05/10] Provide support for outputs and crtcs.
This allow us to size and dynamically resize the dummy driver.
Signed-off-by: Jeremy White
---
spice-video-dummy/src/Makefile.am | 2 +-
spice-video-dummy/src/display.c | 415 ++
spice-video-dummy/src/dri2.c | 5 +-
spice-video-dummy/src/dummy.h | 33 ++
spice-video-dummy/src/dummy_cursor.c | 101 --
spice-video-dummy/src/present.c | 125 ++-
spice-video-dummy/src/spicedummy_driver.c | 53 +--
7 files changed, 589 insertions(+), 145 deletions(-)
create mode 100644 spice-video-dummy/src/display.c
delete mode 100644 spice-video-dummy/src/dummy_cursor.c
diff --git a/spice-video-dummy/src/Makefile.am
b/spice-video-dummy/src/Makefile.am
index 1dc4df8e..8c6e132b 100644
--- a/spice-video-dummy/src/Makefile.am
+++ b/spice-video-dummy/src/Makefile.am
@@ -35,7 +35,7 @@ spicedummy_drv_ladir = @moduledir@/drivers
spicedummy_drv_la_SOURCES = \
compat-api.h \
dri2.c \
- dummy_cursor.c \
spicedummy_driver.c \
+ display.c \
dummy.h \
present.c
diff --git a/spice-video-dummy/src/display.c b/spice-video-dummy/src/display.c
new file mode 100644
index ..b83869bb
--- /dev/null
+++ b/spice-video-dummy/src/display.c
@@ -0,0 +1,415 @@
+/*
+ * Copyright 2019 Jeremy White
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission. The copyright holders make no representations
+ * about the suitability of this software for any purpose. It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "dummy.h"
+
+#define DUMMY_DEFAULT_WIDTH 1024
+#define DUMMY_DEFAULT_HEIGHT 768
+
+static Bool
+crtc_resize(ScrnInfoRec * scrn, int width, int height)
+{
+PixmapRec *pixmap;
+ScreenRec *screen;
+
+screen = xf86ScrnToScreen(scrn);
+pixmap = screen->GetScreenPixmap(screen);
+
+screen->ModifyPixmapHeader(pixmap, width, height, -1, -1, -1, NULL);
+
+scrn->virtualX = width;
+scrn->virtualY = height;
+
+return TRUE;
+}
+
+static const xf86CrtcConfigFuncsRec crtc_config_funcs = {
+crtc_resize
+};
+
+static void
+crtc_dpms(xf86CrtcPtr crtc, int mode)
+{
+xf86DrvMsg(crtc->scrn->scrnIndex, X_INFO, "%s: STUB dpms %d\n", __func__,
mode);
+}
+
+static Bool
+crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode, Rotation rotation,
int x, int y)
+{
+struct dummy_crtc_state *state;
+uint64_t ust;
+
+xf86DrvMsg(crtc->scrn->scrnIndex, X_INFO, "%s: mode %s, x %d, y %d\n",
__func__, mode->name, x,
+ y);
+
+state = crtc->driver_private;
+ust = dummy_gettime_us();
+if (state->interval)
+state->msc_base += ((ust - state->ust_base) / state->interval);
+state->ust_base = ust;
+state->interval =
+(((uint64_t) 1000 * mode->HTotal * mode->VTotal) + (mode->Clock / 2))
/ mode->Clock;
+
+crtc->mode = *mode;
+crtc->x = x;
+crtc->y = y;
+crtc->rotation = rotation;
+#if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC (1, 5, 99, 0, 0)
+crtc->transformPresent = FALSE;
+#endif
+
+return TRUE;
+}
+
+static void
+crtc_destroy(xf86CrtcPtr crtc)
+{
+xf86DrvMsg(crtc->scrn->scrnIndex, X_INFO, "%s\n", __func__);
+
+if (crtc->driver_private) {
+dummy_present_free_vblanks(crtc);
+free(crtc->driver_private);
+}
+crtc->driver_private = NULL;
+}
+
+static void
+dummy_crtc_set_cursor_colors(xf86CrtcPtr crtc, int bg, int fg)
+{
+}
+
+static void
+dummy_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
+{
+}
+
+static void
+dummy_crtc_hide_cursor(xf86CrtcPtr crtc)
+{
+}
+
+static Bool
+dummy_crtc_show_cursor(xf86CrtcPtr crtc)
+{
+return TRUE;
+}
+
+static Bool
+dummy_crtc_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 * image)
+{
+return TRUE;
+}
+
+static const xf86CrtcFuncsRec crtc_funcs =
[Spice-devel] [PATCH x11spice 00/10] Provide a simulated set of outputs and crtcs
The spice-video-dummy driver has two flaws. First, by not providing a crtc, we take the Present extension into a code path where it simulates a refresh rate, and the simulated rate is 1Hz. Applications that query and use that rate then perform badly. You can see that most easily with glxgears. Second, you cannot dynamically resize our dummy driver. By adding a simulated set of outputs and crtcs, we give the dummy driver the ability to control the refresh rate and to allow dynamic resizing. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 08/10] Do not provide a stub client_monitors_config in the QXLInterface.
Using NULL causes the server to relay the message on to the agent,
which does a superior job to anything we currently want to do.
Signed-off-by: Jeremy White
---
src/spice.c | 27 +++
1 file changed, 3 insertions(+), 24 deletions(-)
diff --git a/src/spice.c b/src/spice.c
index 08a5d09a..e9beec01 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -334,29 +334,6 @@ static void update_area_complete(QXLInstance *qin
G_GNUC_UNUSED,
g_debug("TODO: %s UNIMPLEMENTED!", __func__);
}
-static int client_monitors_config(QXLInstance *qin G_GNUC_UNUSED,
- VDAgentMonitorsConfig *monitors_config)
-{
-uint i;
-if (!monitors_config) {
-/* a NULL is used as a test to see if we support this function */
-g_debug("%s: NULL monitors_config", __func__);
-return TRUE;
-}
-
-g_debug("%s: [num %d|flags 0x%x]", __func__,
monitors_config->num_of_monitors,
-monitors_config->flags);
-for (i = 0; i < monitors_config->num_of_monitors; i++)
-g_debug(" %d:[height %d|width %d|depth %d|x %d|y %d]", i,
-monitors_config->monitors[i].height,
-monitors_config->monitors[i].width,
-monitors_config->monitors[i].depth,
-monitors_config->monitors[i].x,
monitors_config->monitors[i].y);
-
-g_debug("TODO: %s UNIMPLEMENTED", __func__);
-return FALSE;
-}
-
/* spice sends AT scancodes (with a strange escape).
* But xf86PostKeyboardEvent expects scancodes. Apparently most of the time
* you just need to add MIN_KEYCODE, see xf86-input-keyboard/src/atKeynames
@@ -545,7 +522,9 @@ void initialize_spice_instance(spice_t *s)
.flush_resources = flush_resources,
.async_complete = async_complete,
.update_area_complete = update_area_complete,
-.client_monitors_config = client_monitors_config,
+.client_monitors_config = NULL, /* Specifying NULL here causes
+ the better logic in the agent
+ to operate */
.set_client_capabilities = NULL,/* Allowed to be unset */
};
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 09/10] Explicitly call ConfigNotify() on the root window in crtc_resize().
From: Henri Verbeet So that any GL clients on the root window (e.g. compositors) know they need to resize their buffers. Is this a hack? It seems like it; other drivers don't appear to need to explicitly call this. Without it though, compositors don't handle screen resizes well. Signed-off-by: Jeremy White --- spice-video-dummy/src/display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spice-video-dummy/src/display.c b/spice-video-dummy/src/display.c index b83869bb..786e6916 100644 --- a/spice-video-dummy/src/display.c +++ b/spice-video-dummy/src/display.c @@ -43,6 +43,8 @@ crtc_resize(ScrnInfoRec * scrn, int width, int height) scrn->virtualX = width; scrn->virtualY = height; +screen->ConfigNotify(screen->root, 0, 0, width, height, screen->root->borderWidth, None); + return TRUE; } -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 10/10] Do not use show_cursor_check on older Xorg builds.
Versions prior to 1.20 do not have the show_cursor_check function.
Signed-off-by: Jeremy White
---
spice-video-dummy/src/display.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/spice-video-dummy/src/display.c b/spice-video-dummy/src/display.c
index 786e6916..31be0416 100644
--- a/spice-video-dummy/src/display.c
+++ b/spice-video-dummy/src/display.c
@@ -113,10 +113,16 @@ dummy_crtc_hide_cursor(xf86CrtcPtr crtc)
{
}
+#if XF86_CRTC_VERSION > 7
static Bool
+#else
+static void
+#endif
dummy_crtc_show_cursor(xf86CrtcPtr crtc)
{
+#if XF86_CRTC_VERSION > 7
return TRUE;
+#endif
}
static Bool
@@ -141,8 +147,12 @@ static const xf86CrtcFuncsRec crtc_funcs = {
.shadow_destroy = NULL,
.set_cursor_colors = dummy_crtc_set_cursor_colors,
.set_cursor_position = dummy_crtc_set_cursor_position,
+#if XF86_CRTC_VERSION > 7
.show_cursor = NULL,
.show_cursor_check = dummy_crtc_show_cursor,
+#else
+.show_cursor = dummy_crtc_show_cursor,
+#endif
.hide_cursor = dummy_crtc_hide_cursor,
.load_cursor_image = NULL,
.load_cursor_image_check = NULL,
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 06/10] Implement page flips.
From: Henri Verbeet
Signed-off-by: Henri Verbeet
Signed-off-by: Jeremy White
---
spice-video-dummy/src/present.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/spice-video-dummy/src/present.c b/spice-video-dummy/src/present.c
index ad29dc91..40290d70 100644
--- a/spice-video-dummy/src/present.c
+++ b/spice-video-dummy/src/present.c
@@ -162,19 +162,24 @@ dummy_present_flush(WindowRec * window)
static Bool
dummy_present_check_flip(RRCrtcRec * crtc, WindowRec * window, PixmapRec *
pixmap, Bool sync_flip)
{
-return FALSE;
+const ScrnInfoRec *scrn = xf86ScreenToScrn(crtc->pScreen);
+const DUMMYRec *dummy = scrn->driverPrivate;
+
+return !dummy->swCursor;
}
static Bool
dummy_present_flip(RRCrtcRec * crtc, uint64_t event_id,
uint64_t target_msc, PixmapRec * pixmap, Bool sync_flip)
{
-return FALSE;
+glamor_block_handler(crtc->pScreen);
+return dummy_present_queue_vblank(crtc, event_id, target_msc) == Success;
}
static void
dummy_present_unflip(ScreenRec * screen, uint64_t event_id)
{
+glamor_block_handler(screen);
present_event_notify(event_id, 0, 0);
}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 04/10] Enable warnings for spice-video-dummy.
From: Henri Verbeet Signed-off-by: Henri Verbeet Signed-off-by: Jeremy White --- spice-video-dummy/src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spice-video-dummy/src/Makefile.am b/spice-video-dummy/src/Makefile.am index 6befa46b..1dc4df8e 100644 --- a/spice-video-dummy/src/Makefile.am +++ b/spice-video-dummy/src/Makefile.am @@ -25,7 +25,7 @@ # _ladir passes a dummy rpath to libtool so the thing will actually link # TODO: -nostdlib/-Bstatic/-lgcc platform magic, not installing the .a, etc. -AM_CFLAGS = $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) +AM_CFLAGS = $(BASE_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) spicedummy_drv_la_LTLIBRARIES = spicedummy_drv.la spicedummy_drv_la_LDFLAGS = -module -avoid-version -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 07/10] Call xf86DPMSInit() in DUMMYScreenInit().
From: Henri Verbeet
This is probably not strictly needed, but it's easy and gets rid of the
"Xlib: extension "DPMS" missing on display ":2"." message.
Signed-off-by: Henri Verbeet
Signed-off-by: Jeremy White
---
spice-video-dummy/src/spicedummy_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/spice-video-dummy/src/spicedummy_driver.c
b/spice-video-dummy/src/spicedummy_driver.c
index 18e96cec..543a3b45 100644
--- a/spice-video-dummy/src/spicedummy_driver.c
+++ b/spice-video-dummy/src/spicedummy_driver.c
@@ -623,6 +623,8 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
if (!xf86CrtcScreenInit(pScreen))
return FALSE;
+xf86DPMSInit(pScreen, xf86DPMSSet, 0);
+
if (dPtr->glamor && !glamor_init(pScreen, glamor_flags)) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
"Failed to initialise glamor at ScreenInit() time.\n");
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 03/10] Use a proper prototype for CreateWindow() in struct dummyRec.
From: Henri Verbeet
Signed-off-by: Henri Verbeet
Signed-off-by: Jeremy White
---
spice-video-dummy/src/dummy.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index d287a59a..dc4ab92f 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -56,7 +56,7 @@ typedef struct dummyRec {
int cursorFG, cursorBG;
dummy_colors colors[1024];
-Bool (*CreateWindow)(); /* wrapped CreateWindow */
+Bool (*CreateWindow)(WindowRec * window); /* wrapped CreateWindow */
Bool prop;
Bool glamor;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 02/10] Add .dirstamp to .gitignore.
From: Henri Verbeet Signed-off-by: Henri Verbeet Signed-off-by: Jeremy White --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index aee6c93a..7052d3ff 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ install-sh missing .deps +.dirstamp Makefile Makefile.in *.o -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice 01/10] Style: tweak a few spaces to match Spice style.
Signed-off-by: Jeremy White
---
spice-video-dummy/src/dummy.h | 2 +-
spice-video-dummy/src/spicedummy_driver.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index cb8afc37..d287a59a 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -56,7 +56,7 @@ typedef struct dummyRec {
int cursorFG, cursorBG;
dummy_colors colors[1024];
-Bool (*CreateWindow)();/* wrapped CreateWindow */
+Bool (*CreateWindow)(); /* wrapped CreateWindow */
Bool prop;
Bool glamor;
diff --git a/spice-video-dummy/src/spicedummy_driver.c
b/spice-video-dummy/src/spicedummy_driver.c
index 1dbe87b2..1efae473 100644
--- a/spice-video-dummy/src/spicedummy_driver.c
+++ b/spice-video-dummy/src/spicedummy_driver.c
@@ -292,7 +292,7 @@ dummy_enable_glamor(ScrnInfoRec * scrn)
const char *accel_method;
if (((accel_method = xf86GetOptValString(dummy->Options,
OPTION_ACCEL_METHOD))
-&& strcmp(accel_method, "glamor")) || dummy->fd == -1) {
+ && strcmp(accel_method, "glamor")) || dummy->fd == -1) {
xf86DrvMsg(scrn->scrnIndex, X_CONFIG, "Glamor disabled.\n");
return FALSE;
}
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH x11spice] Fix compliation on gcc 4.X.
gcc 4.x warns if you use a { 0 } initialization construct
for a structure with an initial member that is also a struct.
The { } construct is simpler and appears to work on a wider
range of gcc versions.
This is a correction to fdfdf1107be100b983de1bff4beee8e6360f670b
Signed-off-by: Jeremy White
---
src/gui.c| 2 +-
src/listen.c | 2 +-
src/main.c | 2 +-
src/spice.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/gui.c b/src/gui.c
index 88acf5c9..3c26b864 100644
--- a/src/gui.c
+++ b/src/gui.c
@@ -147,7 +147,7 @@ void session_disconnect_client(session_t *session)
int main(int argc, char *argv[])
{
gui_t gui;
-session_t session = { 0 };
+session_t session = { };
setlocale(LC_ALL, "");
gui_create(&gui, &session, argc, argv);
diff --git a/src/listen.c b/src/listen.c
index 452fd81f..76c0798a 100644
--- a/src/listen.c
+++ b/src/listen.c
@@ -117,7 +117,7 @@ int listen_parse(const char *listen_spec, char **addr, int
*port_start, int *por
static int try_port(const char *addr, int port)
{
static const int on = 1, off = 0;
-struct addrinfo ai = { 0 }, *res, *e;
+struct addrinfo ai = { }, *res, *e;
char portbuf[33];
int sock, rc;
diff --git a/src/main.c b/src/main.c
index 71cbb465..890ff133 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,7 +55,7 @@ int main(int argc, char *argv[])
{
int rc;
-session_t session = { 0 };
+session_t session = { };
int display_opened = 0;
int spice_started = 0;
diff --git a/src/spice.c b/src/spice.c
index 08a5d09a..561c85ff 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -474,7 +474,7 @@ static int send_monitors_config(spice_t *s, int w, int h)
int spice_create_primary(spice_t *s, int w, int h, int bytes_per_line, void
*shmaddr)
{
-QXLDevSurfaceCreate surface = { 0 };
+QXLDevSurfaceCreate surface = { };
surface.height = h;
surface.width = w;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] I want to contribute spice html5 client
Hi, I actively maintain spice-html5 and work to review patches and see that they are committed. In the cases where I miss a patch or two, other project members usually do a good job of covering them as well. Your contributions would be welcome. Cheers, Jeremy On 10/23/19 6:10 AM, 조미리 wrote: Hi, I'm interested to develop spice web client. But I don't think spice-html5 project is administrated well, Is the project going well? I found out another spice web client project, eyeOS spice-web-client project. I know these two projects aren't relevant. I want to contribute any project of them. But I don't choose where I start. I'm sorry but I want to know which project contribution are worthwhile. Maybe spice-html5 project isn't administrated by anyone, I think that contribution to eyeOS project betters I would appreciate your reply. ___ 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] [PATCH x11spice 00/10] Provide a simulated set of outputs and crtcs
Ping? Henri has reviewed these, and I could have him send an ack, but I was secretly hoping to get an X expert to review patch 5. Cheers, Jeremy On 9/16/19 2:13 PM, Jeremy White wrote: The spice-video-dummy driver has two flaws. First, by not providing a crtc, we take the Present extension into a code path where it simulates a refresh rate, and the simulated rate is 1Hz. Applications that query and use that rate then perform badly. You can see that most easily with glxgears. Second, you cannot dynamically resize our dummy driver. By adding a simulated set of outputs and crtcs, we give the dummy driver the ability to control the refresh rate and to allow dynamic resizing. ___ 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] Using GitLab
Hey Victor, On 10/31/19 10:46 AM, Victor Toso wrote: Hi list, You might note that the Gitlab activity will increase a bit from now on, hopefully. It was agreed within some SPICE collaborators to give a serious try on using this infrastructure that is available to us. One potential great change and challenge is the usage of merge requests in oppose to patch series over mailing list. I hope the benefits outweigh the downsides in the long run. Derek has been working on a utility to integrate GitLab and a mailing list, for experimentation by Wine. He's just gotten to the point of being ready to try a proof of concept. Would you guys be interested in this? Cheers, Jeremy I invite you to subscribe to notifications of the components you might be interested, to discuss and review issues and merge requests and improve different areas code integration, testing and release. https://gitlab.freedesktop.org/spice Cheers, Victor ___ 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] [PATCH x11spice 1/8] Style: tweak a few spaces to match Spice style.
Series acked and pushed.
Cheers,
Jeremy
On 2/18/20 1:53 PM, Henri Verbeet wrote:
From: Jeremy White
Signed-off-by: Jeremy White
Signed-off-by: Henri Verbeet
---
spice-video-dummy/src/dummy.h | 2 +-
spice-video-dummy/src/spicedummy_driver.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index cb8afc3..d287a59 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -56,7 +56,7 @@ typedef struct dummyRec {
int cursorFG, cursorBG;
dummy_colors colors[1024];
-Bool (*CreateWindow)();/* wrapped CreateWindow */
+Bool (*CreateWindow)(); /* wrapped CreateWindow */
Bool prop;
Bool glamor;
diff --git a/spice-video-dummy/src/spicedummy_driver.c
b/spice-video-dummy/src/spicedummy_driver.c
index 1dbe87b..1efae47 100644
--- a/spice-video-dummy/src/spicedummy_driver.c
+++ b/spice-video-dummy/src/spicedummy_driver.c
@@ -292,7 +292,7 @@ dummy_enable_glamor(ScrnInfoRec * scrn)
const char *accel_method;
if (((accel_method = xf86GetOptValString(dummy->Options, OPTION_ACCEL_METHOD))
-&& strcmp(accel_method, "glamor")) || dummy->fd == -1) {
+ && strcmp(accel_method, "glamor")) || dummy->fd == -1) {
xf86DrvMsg(scrn->scrnIndex, X_CONFIG, "Glamor disabled.\n");
return FALSE;
}
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH x11spice] Fix compliation on gcc 4.X.
Hi,
I'm afraid that does not help on the gcc 4.8.5 on RHEL 7.X, particularly
with structures that contain other structures.
gcc -Wall -DTHREE -o /dev/null -c test.c
test.c: In function ‘handle_sigterm’:
test.c:12:12: warning: missing braces around initializer [-Wmissing-braces]
struct sigaction act = { 0 };
^
test.c:12:12: warning: (near initialization for
‘act.__sigaction_handler’) [-Wmissing-braces]
Attached is the test program.
Cheers,
Jeremy
On 3/11/20 1:27 PM, Uri Lublin wrote:
Hi,
Thanks for the ping on IRC.
On 9/17/19 7:23 PM, Jeremy White wrote:
gcc 4.x warns if you use a { 0 } initialization construct
for a structure with an initial member that is also a struct.
The { } construct is simpler and appears to work on a wider
range of gcc versions.
On my Fedora 31, gcc (version 9.2.1) does complain about {} but not
about {0}
when built with -Wpedantic (see simple program below).
Does {0, } works better for you ?
#include
struct S {int a, b; };
void print_s(struct S* ps)
{
if (!ps) { printf("null\n"); return; }
printf("(%d, %d)\n", ps->a, ps->b);
}
int main ()
{
struct S s = { };
print_s(&s);
return 0;
}
Uri.
This is a correction to fdfdf1107be100b983de1bff4beee8e6360f670b
Signed-off-by: Jeremy White
---
src/gui.c | 2 +-
src/listen.c | 2 +-
src/main.c | 2 +-
src/spice.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/gui.c b/src/gui.c
index 88acf5c9..3c26b864 100644
--- a/src/gui.c
+++ b/src/gui.c
@@ -147,7 +147,7 @@ void session_disconnect_client(session_t *session)
int main(int argc, char *argv[])
{
gui_t gui;
- session_t session = { 0 };
+ session_t session = { };
setlocale(LC_ALL, "");
gui_create(&gui, &session, argc, argv);
diff --git a/src/listen.c b/src/listen.c
index 452fd81f..76c0798a 100644
--- a/src/listen.c
+++ b/src/listen.c
@@ -117,7 +117,7 @@ int listen_parse(const char *listen_spec, char
**addr, int *port_start, int *por
static int try_port(const char *addr, int port)
{
static const int on = 1, off = 0;
- struct addrinfo ai = { 0 }, *res, *e;
+ struct addrinfo ai = { }, *res, *e;
char portbuf[33];
int sock, rc;
diff --git a/src/main.c b/src/main.c
index 71cbb465..890ff133 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,7 +55,7 @@ int main(int argc, char *argv[])
{
int rc;
- session_t session = { 0 };
+ session_t session = { };
int display_opened = 0;
int spice_started = 0;
diff --git a/src/spice.c b/src/spice.c
index 08a5d09a..561c85ff 100644
--- a/src/spice.c
+++ b/src/spice.c
@@ -474,7 +474,7 @@ static int send_monitors_config(spice_t *s, int w,
int h)
int spice_create_primary(spice_t *s, int w, int h, int
bytes_per_line, void *shmaddr)
{
- QXLDevSurfaceCreate surface = { 0 };
+ QXLDevSurfaceCreate surface = { };
surface.height = h;
surface.width = w;
#include
static void handle_sigterm(void)
{
#if defined(ONE)
struct sigaction act = { 0, };
#endif
#if defined(TWO)
struct sigaction act = { };
#endif
#if defined(THREE)
struct sigaction act = { 0 };
#endif
sigaction(SIGTERM, &act, 0);
}
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] SPICE: changing the merge rules - a proposal
Hi, The x11spice project is another example: we have only 4 contributors from the beginning of the year (and a single contributor holding 70% of the commits) and the reviews on the gitlab merge requests have been provided by two people only, each one reviewing the merge requests of the other. As projects become more specific/marginal, it's clear that the number of maintainers is lower. Yet, we have those projects under the same umbrella, and I don't think they should be treated differently. 2 active developers/maintainers can be very healthy, I would say. So do we have a maintenance issue with x11spice? Do we want to move the project out of the "Spice-space" projects for ex? What is the problem exactly? The problem is that any merge request I put in sits there until Frediano comes around to look at it. I do have commit rights, but I have not felt comfortable exercising those unilaterally. I am a fan of the core Spice policy - commits only after review. And I have had patches and MRs sit for very long periods of time with no review. Frediano is right that the websocket change is a good example - it is a material improvement for anyone using the spice-html5 client, and it lingered for years without going in. There were certainly patch sets that were small enough that I think it would have been reasonable for me to just 'time out' and apply them without review. Perhaps I'm not using the 'trivial' policy as I should, and I think Daniel is right that there just aren't enough of us. But another truth is I could have shouted louder and more often and perhaps gotten the websocket patch in faster, but it wasn't impacting me or any of my customers, so I didn't. For the sake of having the projects being able to move forward with a reduced number of contributors/reviewers, the proposal is to *allow* a maintainer to merge a Merge Request without an explicit ack if the three following conditions are met: 1) The Merge Request has been pending for at least 3 weeks without getting new comments 2) The Merge Request submitter has kept asking a review on a weekly basis 3) There are no pending nacks on the Merge Request It's hard to define a delay to bypass a reviewing & consensus rule. In general, it should really be frowned upon. There is clearly more than one person interested and using Spice. If the issue is important, it should be fairly easy to get someone else to look at the issue in a timely manner. If not, there are probably more important/interesting things to do to get other interested. I have to say that I was really startled to find that the C++ change had been merged. I largely don't review spice server patches, as I consider myself a consumer of the spice server, and not expert enough to comment on most patches. To my discredit, I haven't really adapted to the gitlab era; I have not subscribed to MRs in areas outside my projects (x11spice, spice-html5). I also agree with Marc-André that this conversation should have taken place over the mailing list. A merge request is not the appropriate place to discuss that substantive a change. And I *would* have noticed a discussion of that significant a change on the mailing list, and I might have even stirred myself to invest in it enough to give a considered opinion. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Brainstorming help with x11spice on socket permissions across users
Hi all, I'm trying to get x11spice and spice-html5, at least as packaged for Fedora, into a pretty much 'turn key' state. I've got 3 use cases. The first is user A sharing their current desktop, either for themselves, or to get help. That case is largely done, imho, modulo some documentation and perhaps some streamlining. The second is user A getting access to a new session for themselves. I don't feel blocked on this case; the work should be straight forward, if fiddly (I may regret those words; doing a secure 'su' like function out of apache may be harder than I think). The 3rd case, however, has me troubled. This is the case that user A (potentially apache) starts x11spice which then does an xdmcp request to gdm, and eventually supports a log in by user B. This makes it challenging to provide a way for user B to launch a spice agent or a pulseaudio daemon and have it securely connect back to the spice process started by user A. The approach I've used in the past is to have a privileged binary use information from an X atom to adjust socket permissions. But that feels unsatisfying, and it seems to me that this is an area with a lot of modern thinking that I've largely missed. As an added complexity, in the ideal case, you have a vdagent running as user A during the login process, which knows to reap itself and give way to a vdagent launched by user B. I was hoping that others would have modern instincts on how to more correctly implement the third use case. Clue bats or other ideas welcome. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Brainstorming help with x11spice on socket permissions across users
I didn't know you could do that. I suppose the solution is X11 only? It would be nice to have gnome-remote-desktop integration. Though GNOME seems more interested to support RDP these days (having a glib/gobject server library would certainly help them to consider Spice, *hint* ;) Yes, although I'm not sure Wayland support would be hard. The second is user A getting access to a new session for themselves. I don't feel blocked on this case; the work should be straight forward, if fiddly (I may regret those words; doing a secure 'su' like function out of apache may be harder than I think). Multiple user session is tricky. Afaik, this is mostly used for desktop development. The instructions to setup such environmnent change over time and desktop. Did I miss something? What's the use case? The use case is I've got a server I'd like to get access to. I hit a web page, provide my credentials, and I have a full login session. Using xdmcp/gdm has the virtue of going through 'standard' channels. The 3rd case, however, has me troubled. This is the case that user A (potentially apache) starts x11spice which then does an xdmcp request to gdm, and eventually supports a log in by user B. This makes it challenging to provide a way for user B to launch a spice agent or a pulseaudio daemon and have it securely connect back to the spice process started by user A. The approach I've used in the past is to have a privileged binary use information from an X atom to adjust socket permissions. But that feels unsatisfying, and it seems to me that this is an area with a lot of modern thinking that I've largely missed. As an added complexity, in the ideal case, you have a vdagent running as user A during the login process, which knows to reap itself and give way to a vdagent launched by user B. I was hoping that others would have modern instincts on how to more correctly implement the third use case. Clue bats or other ideas welcome. This is systemd/desktop territories, and I don't know what would be the best way to do all that. I would suggest you ask the gnome-remote-desktop & systemd/logind developpers, or other desktop developpers how they plan or not to solve it. Check, thanks. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Brainstorming help with x11spice on socket permissions across users
On 5/26/20 10:44 AM, Frediano Ziglio wrote: I suppose you are talking about the unix socket for vdagent, right? Right, but the same mechanism works well for adding audio for example (but you have pulseaudio write to a socket). Hi all, I'm trying to get x11spice and spice-html5, at least as packaged for Fedora, into a pretty much 'turn key' state. I've got 3 use cases. The first is user A sharing their current desktop, either for themselves, or to get help. That case is largely done, imho, modulo some documentation and perhaps some streamlining. The second is user A getting access to a new session for themselves. I don't feel blocked on this case; the work should be straight forward, if fiddly (I may regret those words; doing a secure 'su' like function out of apache may be harder than I think). I would check for the 2nd case if the session is maintained in case you are using SystemD. I suppose the user could want to launch a background X11 session and disconnect from the system. Yeah, good point. In fact, gdm had a cool bug in stock Fedora 32 that was fixed by a recent update. (If you logged in via xdmcp to any user other than the console user, the console switched to the new user). The 3rd case, however, has me troubled. This is the case that user A (potentially apache) starts x11spice which then does an xdmcp request to gdm, and eventually supports a log in by user B. This makes it challenging to provide a way for user B to launch a spice agent or a pulseaudio daemon and have it securely connect back to the spice process started by user A. The approach I've used in the past is to have a privileged binary use information from an X atom to adjust socket permissions. But that feels unsatisfying, and it seems to me that this is an area with a lot of modern thinking that I've largely missed. As far as I know in the normal (physical) case in case of XDMCP two X11 sessions are involved and X11 client have to reconnect to another session. So for symmetry you should reconnect the client and have separate socket for vdagent. Sockets are associated (permission) to different users and processes are associated to same user. As an added complexity, in the ideal case, you have a vdagent running as user A during the login process, which knows to reap itself and give way to a vdagent launched by user B. I was hoping that others would have modern instincts on how to more correctly implement the third use case. Clue bats or other ideas welcome. Cheers, Jeremy To be honest I don't remember last time I used XDMCP. Yeah, the option I'm leaning towards is discarding that use case. (And, tbh, the point of this exercise it is mostly to increase the overall usability; I don't really have a specific problem I'm trying to solve). Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
