Re: [Spice-devel] [phodav PATCH] spice-webdavd: Don't show client folder, when sharing disabled
Hi - Original Message - After enabling shared folder once, the Spice client folder stays in Nautilus, until the spice-webdavd service si restarted. This patch makes sure, that Spice client folder is disabled when sharing is disabled, or the client disconnects. ACK, I suppose gvfs/nautilus keeps the location in memory though. I guess GNOME could have a better integration with spice shared folder, with spice-specific code there, to be discussed with the maintainers ;) --- spice/spice-webdavd.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c index 742f9c9..7b199a3 100644 --- a/spice/spice-webdavd.c +++ b/spice/spice-webdavd.c @@ -222,6 +222,7 @@ static HANDLE port_handle; #endif static void start_mux_read (GInputStream *istream); +static void mdns_unregister_service (void); static void quit (int sig) @@ -466,6 +467,7 @@ end: g_clear_error (error); } + mdns_unregister_service(); quit (-3); } @@ -614,6 +616,17 @@ end: } static void +mdns_unregister_service (void) +{ + if (mdns_group) +{ + g_debug (MDNS client disconected); + ga_entry_group_reset (mdns_group, NULL); + mdns_service = 0; +} +} + +static void mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) { switch (state) @@ -630,11 +643,7 @@ mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) case GA_CLIENT_STATE_S_COLLISION: case GA_CLIENT_STATE_S_REGISTERING: g_message (MDNS collision); - if (mdns_group) -{ - ga_entry_group_reset (mdns_group, NULL); - mdns_service = 0; -} + mdns_unregister_service(); break; default: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [phodav PATCH] spice-webdavd: Don't show client folder, when sharing disabled
Maybe, the right way with Phodav is by using gnome bugzilla https://bugzilla.gnome.org/enter_bug.cgi?product=phodav Cheers, Victor Toso On Wed, Jul 22, 2015 at 04:51:59PM +0200, Lukas Venhoda wrote: After enabling shared folder once, the Spice client folder stays in Nautilus, until the spice-webdavd service si restarted. This patch makes sure, that Spice client folder is disabled when sharing is disabled, or the client disconnects. --- spice/spice-webdavd.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c index 742f9c9..7b199a3 100644 --- a/spice/spice-webdavd.c +++ b/spice/spice-webdavd.c @@ -222,6 +222,7 @@ static HANDLE port_handle; #endif static void start_mux_read (GInputStream *istream); +static void mdns_unregister_service (void); static void quit (int sig) @@ -466,6 +467,7 @@ end: g_clear_error (error); } + mdns_unregister_service(); quit (-3); } @@ -614,6 +616,17 @@ end: } static void +mdns_unregister_service (void) +{ + if (mdns_group) +{ + g_debug (MDNS client disconected); + ga_entry_group_reset (mdns_group, NULL); + mdns_service = 0; +} +} + +static void mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) { switch (state) @@ -630,11 +643,7 @@ mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) case GA_CLIENT_STATE_S_COLLISION: case GA_CLIENT_STATE_S_REGISTERING: g_message (MDNS collision); - if (mdns_group) -{ - ga_entry_group_reset (mdns_group, NULL); - mdns_service = 0; -} + mdns_unregister_service(); break; default: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [phodav PATCH v2] spice-webdavd: Don't show client folder, when sharing disabled
After enabling shared folder once, the Spice client folder stays in Nautilus, until the spice-webdavd service si restarted. This patch makes sure, that Spice client folder is disabled when sharing is disabled, or the client disconnects. --- Changes since v1: - Fixed build on windows - Forgot to ifdef the new function - Fixed missing space before () - Added error checking --- spice/spice-webdavd.c | 30 +- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c index 742f9c9..c8fb795 100644 --- a/spice/spice-webdavd.c +++ b/spice/spice-webdavd.c @@ -222,6 +222,9 @@ static HANDLE port_handle; #endif static void start_mux_read (GInputStream *istream); +#ifdef WITH_AVAHI +static void mdns_unregister_service (void); +#endif static void quit (int sig) @@ -466,6 +469,9 @@ end: g_clear_error (error); } +#ifdef WITH_AVAHI + mdns_unregister_service (); +#endif quit (-3); } @@ -614,6 +620,24 @@ end: } static void +mdns_unregister_service (void) +{ + GError *error = NULL; + + if (mdns_group) +{ + if (!ga_entry_group_reset (mdns_group, error)) +{ + g_warning (Could not disconnect MDNS service: %s, error-message); + g_clear_error (error); +} + + mdns_service = 0; + g_debug (MDNS client disconected); +} +} + +static void mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) { switch (state) @@ -630,11 +654,7 @@ mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) case GA_CLIENT_STATE_S_COLLISION: case GA_CLIENT_STATE_S_REGISTERING: g_message (MDNS collision); - if (mdns_group) -{ - ga_entry_group_reset (mdns_group, NULL); - mdns_service = 0; -} + mdns_unregister_service (); break; default: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/22/2015 09:34 AM, Greg KH wrote: On Wed, Jul 22, 2015 at 09:03:53AM -0500, Jeremy White wrote: On 07/09/2015 05:06 AM, Alex Elsayed wrote: Alan Stern wrote: On Mon, 6 Jul 2015, Jeremy White wrote: Anything else fundamental to usbip that should inform the design of a usbredir driver? usbip appears to be based off a 2004 vintage of dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. Another thing that comes to mind is that the USB-IF has its own official standard for this kind of thing now, called Media-Agnostic USB[1]. In November of 2014 a driver[2] was posted, followed by a second version[3], and it is apparently being refined inside Intel[4]. [1] http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip [2] http://thread.gmane.org/gmane.linux.kernel/1820297 [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 Thanks for the pointer, Alex. I spent some time with the spec and their proposed code. It does seem plausible that XSpice could use a mausb/usbredir protocol converter. So if there was a mausb kernel module, I could potentially implement support in XSpice in user space and not need a usbredir module. I sent an email to the two developers at Intel to ask if there had been any further progress and if I could collaborate with them. I have not heard back. The MA spec is substantial and seems well thought out. But the usbredir protocol has the virtue of being relatively mature - it's 5 years old, with code in daily use. At this point it seems the best path forward is to continue work on the usbredir kernel module, which I will do unless I get some new information. Please work with the existing people, or with the existing spec, I don't want to be adding multiple versions of this type of protocol to the kernel. As it is, I really don't even want to take your code, given that usbip is already there. Ignoring it isn't ok. The usbredir spec predates MA-USB by 4 years; it has greater claim to the title 'existing' than does MA-USB. I recognize that does not make it better, and I recognize the value of a spec from a standards body. But I also respect community standards in production use. And I did not and am not ignoring the MA-USB patch and spec. I privately wrote to the Intel authors of that patch a week ago; I've publicly included them in this thread as well. As far as I can tell, they've been silent on this front since November; I fear that they may have moved on, or that Intel is not actively working on this. None of the Intel authors listed on the MA-USB specification are kernel contributors, so I did not have a way to reach out to them. If you have the means to engage others, I would appreciate that. With no other input, my analysis was that it is better to proceed with the existing spec. It has a body of useful code, active users and developers, and I am certain it will solve my problem. Also, as for usbip, I'll point out that the existence of MA-USB corroborates Hans rationale for the need to supplant usbip. As I said, I will respond to any new information I receive. It would be great to have a kernel module developed (or at least approved) by seasoned hands at Intel. But how long should I wait? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector
Generate global definitions. Generate function to registers various dissector components. For the moment the field array is empty but we save some global to be able to register new ones. Add a base test for generated dissector. Signed-off-by: Frediano Ziglio fzig...@redhat.com --- Makefile.am | 2 +- codegen/Makefile.am | 42 ++ codegen/dissector_test.c| 81 + configure.ac| 5 +++ python_modules/dissector.py | 87 +++-- spice_codegen.py| 2 +- 6 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 codegen/Makefile.am create mode 100644 codegen/dissector_test.c Changes from v3: - do not fail compiling if wireshark is not found; - use BUILT_SOURCES to generate sources instead of manual dependency; - fix typo in comment. diff --git a/Makefile.am b/Makefile.am index 380bf24..382a0ea 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,7 +1,7 @@ NULL = ACLOCAL_AMFLAGS = -I m4 -SUBDIRS = python_modules common +SUBDIRS = python_modules common codegen DIST_SUBDIRS = spice-protocol $(SUBDIRS) EXTRA_DIST = \ diff --git a/codegen/Makefile.am b/codegen/Makefile.am new file mode 100644 index 000..2bbfc2a --- /dev/null +++ b/codegen/Makefile.am @@ -0,0 +1,42 @@ +NULL = + +if WIRESHARK +AM_CPPFLAGS = \ + -I$(top_srcdir) \ + $(WIRESHARK_CFLAGS) \ + $(SPICE_COMMON_CFLAGS) \ + $(NULL) + +dissector_test_LDADD = \ + $(SPICE_COMMON_LIBS)\ + $(NULL) + +MARSHALLERS_DEPS = \ + $(top_srcdir)/python_modules/__init__.py\ + $(top_srcdir)/python_modules/codegen.py \ + $(top_srcdir)/python_modules/demarshal.py \ + $(top_srcdir)/python_modules/marshal.py \ + $(top_srcdir)/python_modules/ptypes.py \ + $(top_srcdir)/python_modules/spice_parser.py\ + $(top_srcdir)/python_modules/dissector.py \ + $(top_srcdir)/spice_codegen.py \ + $(NULL) + +BUILT_SOURCES = test.h + +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ $@ /dev/null + +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ --header $@ /dev/null + +TESTS = dissector_test +check_PROGRAMS = dissector_test + +dissector_test_SOURCES = dissector_test.c test.c test.h +endif + +EXTRA_DIST = \ + $(NULL) + +-include $(top_srcdir)/git.mk diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c new file mode 100644 index 000..6189da0 --- /dev/null +++ b/codegen/dissector_test.c @@ -0,0 +1,81 @@ +#undef NDEBUG +#include stdarg.h +#include stdio.h +#include stdlib.h +#include unistd.h +#include getopt.h +#include assert.h + +#include epan/expert.h + +#include test.h + +enum { + first_hf_registered = 0x1, + first_ei_registered = 0x2, +}; +static int last_hf_registered = first_hf_registered - 1; +static int last_ei_registered = first_ei_registered - 1; + +WS_DLL_PUBLIC void +proto_register_field_array(const int parent, hf_register_info *hf, const int num_records) +{ + int i; + assert(num_records = 0); + for (i = 0; i num_records; ++i) { + assert(hf); + assert(hf[i].p_id); + assert(*hf[i].p_id == -1); + *hf[i].p_id = ++last_hf_registered; + } +} + +WS_DLL_PUBLIC void +expert_register_field_array(expert_module_t* module, ei_register_info *ei, const int num_records) +{ + int i; + assert(num_records = 0); + for (i = 0; i num_records; ++i) { + assert(ei ei[i].ids-ei == -1); + ei[i].ids-ei = ++last_ei_registered; + } +} + +static const struct option long_options[] = { + { help, 0, NULL, 'h' }, +}; +static const char options[] = h; + +static void syntax(FILE *f, int exit_value) +{ + fprintf(f, + Usage: dissector_test [OPTIONS]\n + \n + Options:\n + -h, --help Show this help\n + ); + exit(exit_value); +} + +int main(int argc, char **argv) +{ + while (1) { + int option_index; + int opt = getopt_long(argc, argv, options, long_options, option_index); + if (opt == -1) + break; + + switch (opt) { + case 'h': + syntax(stdout, EXIT_SUCCESS); + break; + default: + syntax(stderr, EXIT_FAILURE); + break; + } + } + +
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Tue, Jul 21, 2015 at 04:36:45PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:34:08AM -0400, Laine Stump wrote: On 07/21/2015 09:41 AM, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 03:35:50PM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. It isn't sufficient to just differentiate loading on startup vs defining them IIUC. We need to differentiate loading on startup from XML previously written by a libvirtd X.Y.Z which is why I think we need to have a version number recorded in the XML ultimately. But a version number isn't sufficient to describe exactly what the previous libvirt was capable of. Many times new features (externally visible only in the XML) are backported to earlier versions of libvirt downstream (e.g. libvirt-0.10.2 that is used in RHEL6.x and CentOS 6.x), so this still doesn't buy us perfection. Downstream maintainers could make it better by paying very close attention to any use of this version number when backporting new stuff, but there would still be problems if someone decided to build and install an upstream libvirt on a system that previously had some hybrid downstream build. (Not saying we shouldn't do it, just that it's no perfect :-) Yep, understood. I'm thinking purely in terms of upstream where we do not backport features to stable branches. Distros which get into the feature backport game have to deal with that pain themselves. How about we use some part of the XML to mark features there? That part would only be parsed on loading and formatted into migration cookie and the XML saved on disk. We can put it into another namespace. Each feature would have its handler that fixes whatever needs to be fixed in the postparse part. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector
On Tue, Jul 21, 2015 at 11:54:06AM -0400, Frediano Ziglio wrote: From: Christophe Fergeau cferg...@redhat.com This test.o dep (and the similar one in another commit) is odd. Missing BUILT_SOURCES? No, it's just manual, see http://www.delorie.com/gnu/docs/automake/automake_69.html and http://www.delorie.com/gnu/docs/automake/automake_70.html, basically not all cases are covered by automatic generated dependency. Stuff like Adding explicit dependencies like this can be a bit dangerous if you are not careful enough. Always check the generated `Makefile.in' if you do this. would make me strongly favour use of BUILT_SOURCES, especially as make in the codegen directory will only generate the needed sources (make check will then build the code). + +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ $@ /dev/null + +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $ --header $@ /dev/null + +TESTS = dissector_test +check_PROGRAMS = dissector_test + +dissector_test_SOURCES = dissector_test.c test.c test.h + +EXTRA_DIST = \ + $(NULL) + +-include $(top_srcdir)/git.mk diff --git a/configure.ac b/configure.ac index 4287f92..a156cae 100644 --- a/configure.ac +++ b/configure.ac @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON) SPICE_CHECK_SMARTCARD(SPICE_COMMON) SPICE_CHECK_CELT051(SPICE_COMMON) SPICE_CHECK_GLIB2(SPICE_COMMON) +PKG_CHECK_MODULES(WIRESHARK, wireshark) This should be optional. Oh... is it not in this way? I though so. Nope, this errors out if wireshark is not found (easy to test by changing 'wireshark' to something random). If you want a --enable-dissector argument to configure, you can take a look at m4/spice-deps.m4 for an example of how to handle it (defaults to auto-detection, errors out if --enable-dissector is passed but wireshark is not found, always disables it if --disable-dissector is passed). Here, I think just doing PKG_CHECK_MODULES(WIRESHARK, [have_wireshark=yes], [have_wireshark=no]) AM_CONDITIONAL([HAVE_WIRESHARK], [test x$have_wireshark = xyes]) should be enough (and then add the appropriate ifdef to the Makefile.am) Christophe pgpb49atIOvIT.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common] codegen: Fix enums.h generation with python3
Trying to generate enums.h with python3 results in Traceback (most recent call last): File ./spice_codegen.py, line 217, in module write_enums(writer, options.generate_dissector) File ./spice_codegen.py, line 99, in write_enums write_channel_enums(writer, c, False, False) File ./spice_codegen.py, line 17, in write_channel_enums if len(messages) == 0: TypeError: object of type 'filter' has no len() filter() returns an enumerator object in python3 while it used to return a list in python2. Using list(filter()) instead fixes that error. I've checked that the generated enums.h is identical with python2 and python3. --- spice_codegen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spice_codegen.py b/spice_codegen.py index 84790af..569 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -12,8 +12,8 @@ from python_modules import marshal import six def write_channel_enums(writer, channel, client, describe): -messages = filter(lambda m : m.channel == channel, \ - channel.client_messages if client else channel.server_messages) +messages = list(filter(lambda m : m.channel == channel, \ + channel.client_messages if client else channel.server_messages)) if len(messages) == 0: return if client: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Wed, Jul 22, 2015 at 09:59:00AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 04:36:45PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:34:08AM -0400, Laine Stump wrote: On 07/21/2015 09:41 AM, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 03:35:50PM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. It isn't sufficient to just differentiate loading on startup vs defining them IIUC. We need to differentiate loading on startup from XML previously written by a libvirtd X.Y.Z which is why I think we need to have a version number recorded in the XML ultimately. But a version number isn't sufficient to describe exactly what the previous libvirt was capable of. Many times new features (externally visible only in the XML) are backported to earlier versions of libvirt downstream (e.g. libvirt-0.10.2 that is used in RHEL6.x and CentOS 6.x), so this still doesn't buy us perfection. Downstream maintainers could make it better by paying very close attention to any use of this version number when backporting new stuff, but there would still be problems if someone decided to build and install an upstream libvirt on a system that previously had some hybrid downstream build. (Not saying we shouldn't do it, just that it's no perfect :-) Yep, understood. I'm thinking purely in terms of upstream where we do not backport features to stable branches. Distros which get into the feature backport game have to deal with that pain themselves. How about we use some part of the XML to mark features there? That part would only be parsed on loading and formatted into migration cookie and the XML saved on disk. We can put it into another namespace. Each feature would have its handler that fixes whatever needs to be fixed in the postparse part. I'm not sure what you are suggesting by features here, but I'd prefer if we didn't introduce a chunk of XML which would contain an ever growing set of hacks. It seems sufficient for us to just record the libvirt version number which generated the XML document, so newer libvirt can detect a previous buggy version and correct what's needed, so the hacks are confined to the code and not spread across code + xml. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On Wed, Jul 22, 2015 at 10:14:27AM +0100, Daniel P. Berrange wrote: I'm not sure what you are suggesting by features here, but I'd prefer if we didn't introduce a chunk of XML which would contain an ever growing set of hacks. It seems sufficient for us to just record the libvirt version number which generated the XML document, so newer libvirt can detect a previous buggy version and correct what's needed, so the hacks are confined to the code and not spread across code + xml. Seems reasonable. Moreover, the fixes can be handled in a way that back-porting doesn't cause problems as Laine was worried. I.e. the XML would be fixed twice. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH v4] qemu: Use heads parameter for QXL driver
On 07/21/2015 09:41 AM, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 03:35:50PM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 01:50:22PM +0100, Daniel P. Berrange wrote: On Tue, Jul 21, 2015 at 11:44:27AM +0200, Martin Kletzander wrote: On Tue, Jul 21, 2015 at 09:36:55AM +0200, Christophe Fergeau wrote: On Mon, Jul 20, 2015 at 11:25:52AM +0200, Martin Kletzander wrote: I spend all morning fixing this to be installed properly in the system. Anyway, I finally managed to make this work and found out the guest I used for it is not ready to have multiple monitors. Anyway, looking at everything else this definitely works, so ACK, I'll push it in a while. I'm still a bit worried that all existing guests will have heads='1' in their XML as libvirt is adding that by default, but I don't really see a way around it :-/ We should make sure libvirt stops adding it from now on though ;) Well, how would you then limit the outputs to 1? Having said that, I have no idea why we started adding heads=1 automatically and playing with different changes, we have another bug in the parsing/formatting code. You'll also won't be able to migrate from older libvirt with heads='1' to newer ones. I haven't realized that. I'm thinking about reverting the change in libvirt or even using heads 1. Although that won't migrate either. So the only other thing that we can do is to introduce new parameter, like maxHeads. The sound you just heard is me slapping my face because again there will multiple parameters meaning similar things... Introducing a new parameter is not really viable IMHO, because as you say it'll leave us with two things meaning the same, and will not be compatible with existing apps that know about the current parameter. I think we need to figure out a way to drop the 'heads=1' from any existing configs in /etc/libvirt/qemu when we start up with the new libvirtd for the first time. I'm already working on an RFC that would differentiate between loading XMLs on daemon start-up and defining them. The purpose of that is so that we can make incompatible XML changes without losing any domains, but that's not the point here. If we were to drop heads='1' anywhere, this is the place. The ABI change would be minimal for the guest. It isn't sufficient to just differentiate loading on startup vs defining them IIUC. We need to differentiate loading on startup from XML previously written by a libvirtd X.Y.Z which is why I think we need to have a version number recorded in the XML ultimately. But a version number isn't sufficient to describe exactly what the previous libvirt was capable of. Many times new features (externally visible only in the XML) are backported to earlier versions of libvirt downstream (e.g. libvirt-0.10.2 that is used in RHEL6.x and CentOS 6.x), so this still doesn't buy us perfection. Downstream maintainers could make it better by paying very close attention to any use of this version number when backporting new stuff, but there would still be problems if someone decided to build and install an upstream libvirt on a system that previously had some hybrid downstream build. (Not saying we shouldn't do it, just that it's no perfect :-) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common] codegen: Fix enums.h generation with python3
On Wed, Jul 22, 2015 at 12:51 PM, Christophe Fergeau cferg...@redhat.com wrote: Trying to generate enums.h with python3 results in Traceback (most recent call last): File ./spice_codegen.py, line 217, in module write_enums(writer, options.generate_dissector) File ./spice_codegen.py, line 99, in write_enums write_channel_enums(writer, c, False, False) File ./spice_codegen.py, line 17, in write_channel_enums if len(messages) == 0: TypeError: object of type 'filter' has no len() filter() returns an enumerator object in python3 while it used to return a list in python2. Using list(filter()) instead fixes that error. I've checked that the generated enums.h is identical with python2 and python3. --- spice_codegen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spice_codegen.py b/spice_codegen.py index 84790af..569 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -12,8 +12,8 @@ from python_modules import marshal import six def write_channel_enums(writer, channel, client, describe): -messages = filter(lambda m : m.channel == channel, \ - channel.client_messages if client else channel.server_messages) +messages = list(filter(lambda m : m.channel == channel, \ + channel.client_messages if client else channel.server_messages)) if len(messages) == 0: return if client: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ACK! -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/22/2015 12:59 PM, Sean O. Stalley wrote: On Wed, Jul 22, 2015 at 11:55:32AM -0500, Jeremy White wrote: I privately wrote to the Intel authors of that patch a week ago; I've publicly included them in this thread as well. As far as I can tell, they've been silent on this front since November; I fear that they may have moved on, or that Intel is not actively working on this. None of the Intel authors listed on the MA-USB specification are kernel contributors, so I did not have a way to reach out to them. If you have the means to engage others, I would appreciate that. Sorry for the delay. The short answer is: Yes, we have been actively working on this driver. Per Greg KH's request, we have been cleaning up the driver internally. There was a lot to clean up, which is why we have been silent on LKML. Great, thanks. I would appreciate a chance to work with you to make sure it will work well for XSpice. I'm happy to help as much as I can, and please don't feel the need to wait for a final version before reaching out to me. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, Jul 22, 2015 at 09:03:53AM -0500, Jeremy White wrote: On 07/09/2015 05:06 AM, Alex Elsayed wrote: Alan Stern wrote: On Mon, 6 Jul 2015, Jeremy White wrote: Anything else fundamental to usbip that should inform the design of a usbredir driver? usbip appears to be based off a 2004 vintage of dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. Another thing that comes to mind is that the USB-IF has its own official standard for this kind of thing now, called Media-Agnostic USB[1]. In November of 2014 a driver[2] was posted, followed by a second version[3], and it is apparently being refined inside Intel[4]. [1] http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip [2] http://thread.gmane.org/gmane.linux.kernel/1820297 [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 Thanks for the pointer, Alex. I spent some time with the spec and their proposed code. It does seem plausible that XSpice could use a mausb/usbredir protocol converter. So if there was a mausb kernel module, I could potentially implement support in XSpice in user space and not need a usbredir module. I sent an email to the two developers at Intel to ask if there had been any further progress and if I could collaborate with them. I have not heard back. The MA spec is substantial and seems well thought out. But the usbredir protocol has the virtue of being relatively mature - it's 5 years old, with code in daily use. At this point it seems the best path forward is to continue work on the usbredir kernel module, which I will do unless I get some new information. Please work with the existing people, or with the existing spec, I don't want to be adding multiple versions of this type of protocol to the kernel. As it is, I really don't even want to take your code, given that usbip is already there. Ignoring it isn't ok. thanks, greg k-h ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [phodav PATCH] spice-webdavd: Don't show client folder, when sharing disabled
After enabling shared folder once, the Spice client folder stays in Nautilus, until the spice-webdavd service si restarted. This patch makes sure, that Spice client folder is disabled when sharing is disabled, or the client disconnects. --- spice/spice-webdavd.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c index 742f9c9..7b199a3 100644 --- a/spice/spice-webdavd.c +++ b/spice/spice-webdavd.c @@ -222,6 +222,7 @@ static HANDLE port_handle; #endif static void start_mux_read (GInputStream *istream); +static void mdns_unregister_service (void); static void quit (int sig) @@ -466,6 +467,7 @@ end: g_clear_error (error); } + mdns_unregister_service(); quit (-3); } @@ -614,6 +616,17 @@ end: } static void +mdns_unregister_service (void) +{ + if (mdns_group) +{ + g_debug (MDNS client disconected); + ga_entry_group_reset (mdns_group, NULL); + mdns_service = 0; +} +} + +static void mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) { switch (state) @@ -630,11 +643,7 @@ mdns_state_changed (GaClient *client, GaClientState state, gpointer user_data) case GA_CLIENT_STATE_S_COLLISION: case GA_CLIENT_STATE_S_REGISTERING: g_message (MDNS collision); - if (mdns_group) -{ - ga_entry_group_reset (mdns_group, NULL); - mdns_service = 0; -} + mdns_unregister_service(); break; default: -- 2.4.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/09/2015 05:06 AM, Alex Elsayed wrote: Alan Stern wrote: On Mon, 6 Jul 2015, Jeremy White wrote: Anything else fundamental to usbip that should inform the design of a usbredir driver? usbip appears to be based off a 2004 vintage of dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. Another thing that comes to mind is that the USB-IF has its own official standard for this kind of thing now, called Media-Agnostic USB[1]. In November of 2014 a driver[2] was posted, followed by a second version[3], and it is apparently being refined inside Intel[4]. [1] http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip [2] http://thread.gmane.org/gmane.linux.kernel/1820297 [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 Thanks for the pointer, Alex. I spent some time with the spec and their proposed code. It does seem plausible that XSpice could use a mausb/usbredir protocol converter. So if there was a mausb kernel module, I could potentially implement support in XSpice in user space and not need a usbredir module. I sent an email to the two developers at Intel to ask if there had been any further progress and if I could collaborate with them. I have not heard back. The MA spec is substantial and seems well thought out. But the usbredir protocol has the virtue of being relatively mature - it's 5 years old, with code in daily use. At this point it seems the best path forward is to continue work on the usbredir kernel module, which I will do unless I get some new information. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [libvirt] [PATCH] qxl: Fix new function name for spice-server library
Hi On Mon, Jul 20, 2015 at 11:46 AM, Martin Kletzander mklet...@redhat.com wrote: On Mon, Jul 20, 2015 at 09:43:23AM +0100, Frediano Ziglio wrote: The new spice-server function to limit the number of monitors (0.12.6) changed while development from spice_qxl_set_monitors_config_limit to spice_qxl_max_monitors (accepted upstream). By mistake I post patch with former name. This patch fix the function name. Signed-off-by: Frediano Ziglio fzig...@redhat.com ACK, I'll try sending a proper pull request for 2.4 since Gerd is on holidays --- hw/display/qxl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) I tested again doing a clean build, unfortunately I did some mistake and my tests worked. diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 4e5ff69..2288238 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -273,8 +273,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay) } else { #if SPICE_SERVER_VERSION = 0x000c06 /* release 0.12.6 */ if (qxl-max_outputs) { -spice_qxl_set_monitors_config_limit(qxl-ssd.qxl, -qxl-max_outputs); +spice_qxl_set_max_monitors(qxl-ssd.qxl, qxl-max_outputs); } #endif qxl-guest_monitors_config = qxl-ram-monitors_config; -- 2.1.0 H Same as the fix I did in order for this to work with upstream spice. ACK. Weak, though, as I'm not a privileged one. Martin ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel