Re: [Spice-devel] [ovirt-devel] ovirt-guest-agent behavior on disconnect

[Michal Skrivanek]

On Sep 25, 2015, at 19:40 , David Mansfield  wrote:

> [cross-posted to de...@ovirt.org and spice-devel@lists.freedesktop.org]
> 
> Hi oVirt Devs,
> 
> I'm here from the spice-devel list where we were discussing some changes to 
> the behavior of the spice guest agent reacting to a user disconnect (of the 
> spice console).

Hi David,
great, any enhancement is good! Vinzenz, please add more details to my guesses 
below:)

> 
> Some information about how the ovirt-guest-agent works would be informative 
> if you can spare a minute.
> 
> The functionality being discussed is locking the user session in the VM when 
> the user disconnects from spice (either intentionally or unintentionally).
> 
> Also, peripherally, how does oVirt ensure secure access by authorized users 
> of a VM and prevent "over-the-shoulder" snooping (spice graphics session 
> stealing) or other forms of information leak from a VM shared by multiple 
> users.
> 
> So here are some questions:
> 
> Can a VM be "shared" by multiple users in oVirt at all? Are there known 
> security issues that would make this a non-recommended or fundamentally 
> un-securable setup?

normally no, there is a semi-supported hook to allow that with VNC (and even 
that is slightly broken IIRC at the moment), but in general we do want so 
support that for specific usecases

> 
> Does the oVirt agent lock the session on disconnect? Always / 
> unconditionally? If it's configurable, where does the configuration reside - 
> in the vm guest, on the vm host (/engine) or on the client?

it's oVirt management UI configuration, it changes the host's behavior on spice 
disconnect per VM

> 
> Does the oVirt agent lock all sessions or the current active session?

just the active AFAIK

> 
> How does it lock the sessions?  I've looked at the code and it appears 
> '/usr/bin/loginctl lock-sessions' is being used on machines it's provided on 
> and something more complicated on older boxes.  Does the user have a way to 
> customize this behavior? and if so, is it VM guest, VM host or client 
> configuration?

> 
> Does the agent lock linux consoles (VC1, VC2) "sessions" (e.g. with vlock?)
> 
> As I understand it, console access in ovirt is managed by setting a temporary 
> graphics password and then generating an .ini file which is launched by 
> remote-viewer. This password expires after a short period of time.  So is 
> there a mechanism where access is denied if a user is already connected or is 
> this allowed?

connection is not allowed unless "strict user checking" disabled in UI
if it is disable or you use the same pwd then the previous session is 
terminated and replaced (unless using that hook I mentioned).
But we try to treat the .vv file as a one time thing, there's 
delete_this_file=1 which instructs virt-viewer to remove the file upon startup, 
so even when browser place them on a shared drive they shouldn't be there for 
too long


What kind of changes do you have in mind on the SPICE side?
It would certainly make it easier for us as currently we kind of guess when to 
lock…we receive multiple disconnecst(per channel) and don't really know what's 
going on…having a direct support for this inside the spice server would be 
better. But it needs to allow the flexibility of different actions except 
desktop lock (we have "nothing", "shutdown", "logoff" I think). Perhaps a way 
how to signal relevant information to vdsm is enough

Thanks,
michal

> 
> Enough questions for now, sorry for the battering.
> 
> -- 
> Thanks,
> David Mansfield
> Cobite, INC.
> ___
> Devel mailing list
> de...@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] 回复：Re: [help]there's an error during i compile spice-gtk code

[hongzhen_luo]

- 原始邮件 -
发件人：Christophe Fergeau 
收件人：hongzhen_...@sina.com
抄送人：spice-devel 
主题：Re: [Spice-devel] [help]there's an error during i compile spice-gtk code
日期：2015年09月28日 15点34分

On Mon, Sep 28, 2015 at 08:57:19AM +0800, hongzhen_...@sina.com wrote:
> Dear Sir   Thanks for you help, I have successful compiled my code .  
>  About the new interface spice_reget_usb_state(), it's for a bug  
>   :that's  when I activated the usb device widget to click the check
>  box for installing usb driver ,then closed(click X button) the device 
> widget quickly and run the device widget again . 
>Normally ,the mark of checkbox should be remained ,but it's 
> disappeared. So when you click again ,it will be popped up a message box
>  : "Operation not supported or unimplemented on this platform" So I 
> checked the source code and found in the function : device_added_cb()   
> if (spice_usb_device_manager_is_device_connected(priv->manager,   
>   device))
> gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check), TRUE);   As,
>  you see ,the condition was whether the device was connected ,if true 
> the checkbox was set checked ,maybe for some reasons ,such as network 
> delay or others ,  the usb needs more long time to redirect at Linux 
> ,however ,this interface(spice_usb_device_manager_is_device_connected)  just 
> do judgement that it's whether has been connected ,probably
>  .So when I operated quickly at device widget  (click checkbox -close 
> widget -run widget -click check box) the mark was disappeared .  So  i 
> want to add a  condition to get the current usb state 
> ,if(spice_usb_device_manager_is_device_connected ||state == 
> SPICE_USB_DEVICE_STATE_INSTALLING) {then .} so  i write a method 
> to get the state .  I didn't test the result yet ..maybe it will be 
> successful maybe not ..
Ah ok, thanks for  the details, it indeed looks like the handling of
this INSTALLING state is currently missing in device_added_cb().
Christophe
Dear Mr Christophe  unfortunately, the test result was failed , when I 
added the contidion SPICE_USB_DEVICE_STATE_INSTALLING  , the USB device is 
always keeped  'check' state . In the usb-device-manager I just wrote a 
function() to call the static method 'spice_usb_device_get_state()' then obtain 
the return value to usb-device -widget .. but i don't know why it always keep 
installing state  or I used incorrect value to compare in my condition  ..Do 
you know how to modify this bug ? 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 1/2] Update NEWS

[Christophe Fergeau]

On Tue, Sep 29, 2015 at 11:53:00AM -0500, Jonathon Jongsma wrote:
> On Tue, 2015-09-29 at 18:00 +0200, Christophe Fergeau wrote:
> > ---
> >  NEWS | 10 ++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/NEWS b/NEWS
> > index 2fbac0d..ff4cf3a 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,3 +1,13 @@
> > +Major changes in 0.12.6:
> > +
> > +* Remove write polling in chardevs to reduce wakeups
> > +* Unix socket support
> > +* LZ4 support
> > +* Let clients specify their preferred image compression format
> > +* Allow to record and replay a spice-server session
> > +* Fix for CVE-2015-3247
> > +* Various bugs, crashes fixes, code cleanups, ...
> 
> - Mention the removal of old client code?
> - mention new external spice-protocol requirement?

Ah thanks, missed both, I'll add these.

> - There seems to be quite a few endianness fixes that might be of
> interest to non-x86 people. Worth mentioning?

More patches are most likely needed for a working big-endian
spice-server, that's why I omitted them. Does anyone know how much of
spice is working on PPC?

Christophe


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 v5 16/20] spice-gtk: Refactor the video decoding to use a more object oriented design.

[Francois Gouget]

On Mon, 21 Sep 2015, Christophe Fergeau wrote:
[...]
> > -st->msg_data = in;
> 
> Not very clear in this commit why you needed to stop storing the message
> within the display_stream structure, and makes the diff bigger than it
> could, I guess I'll find an explanation when looking at the commits
> introducing gstreamer ;)

This patch introduces a proper API for the video decoder. Part of that 
is not having the video decoder depend on the internals of the 
display_stream structure so it is more reusable.

Before this patch channel-display-mjpeg.c has no place to store the 
frame message. Yet, as far as I can see, it cannot pass it to the 
mjpeg_src_init() callback that needs it. So removing the 
display_stream.msg_data field cannot come before this patch. And I don't 
think it's worth introducing a flawed version of the video decoder API 
just to fix it in the next patch.

-- 
Francois Gouget 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 1/2] Update NEWS

[Jonathon Jongsma]

On Wed, 2015-09-30 at 11:12 +0200, Christophe Fergeau wrote:
> On Tue, Sep 29, 2015 at 11:53:00AM -0500, Jonathon Jongsma wrote:
> > On Tue, 2015-09-29 at 18:00 +0200, Christophe Fergeau wrote:
> > > ---
> > >  NEWS | 10 ++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/NEWS b/NEWS
> > > index 2fbac0d..ff4cf3a 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -1,3 +1,13 @@
> > > +Major changes in 0.12.6:
> > > +
> > > +* Remove write polling in chardevs to reduce wakeups
> > > +* Unix socket support
> > > +* LZ4 support
> > > +* Let clients specify their preferred image compression format
> > > +* Allow to record and replay a spice-server session
> > > +* Fix for CVE-2015-3247
> > > +* Various bugs, crashes fixes, code cleanups, ...
> > 
> > - Mention the removal of old client code?
> > - mention new external spice-protocol requirement?
> 
> Ah thanks, missed both, I'll add these.
> 
> > - There seems to be quite a few endianness fixes that might be of
> > interest to non-x86 people. Worth mentioning?
> 
> More patches are most likely needed for a working big-endian
> spice-server, that's why I omitted them. Does anyone know how much of
> spice is working on PPC?

Yeah, I'm not sure how well it works. Probably OK to leave them off
then.

Jonathon

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v5 07/20] server: Let the video encoder manage the compressed buffer and avoid copying it.

[Francois Gouget]

On Tue, 22 Sep 2015, Christophe Fergeau wrote:
[...]
> What I'm getting at is that VideoBuffer seems to only be used from 
> red_marshall_stream_data(),
> and that all that the non-VideoEncoder code needs is
> - some data buffer
> - the size of that data buffer
> - some opaque pointer which manages this data
> - a function to free the opaque pointer when it's no longer needed.
> 
> It seems to me that VideoBuffer could be stack-allocated (or even
> removed), and that the gstreamer backend could just return a directly
> GstBuffer with gst_buffer_unref() as its free function, rather than
> what is done currently where you create a GstVideoBuffer wrapping a
> GstBuffer, and then you have some more code to try and reuse existing
> GstVideoBuffer instances rather than allocating a new one.
> 
> The GStreamer 1.0 situation is a bit more complicated as you need a
> GstBuffer and a GstMapInfo in the free function, I think I would use
> gst_mini_object_set_qdata() to attach the GstMapInfo data to the
> GstBuffer.

I don't think getting rid of the VideoBuffer structure is a good idea. 
Instead of encode_frame() returning one easily extended structure it 
will have to return four values: a pointer to the data, the data size, 
an opaque pointer and a callback. And the video encoder may still have 
to allocate a structure to store all the information it needs to 
properly release the frame buffer, as is the case for GStreamer 1.0.


> The needed data, data size, opaque pointer, free func could
> alternatively be returned by a new encoder vfunc,

This would only work if we assume that encode_frame() cannot called 
until the previous compressed frame has been released, or force adding a 
way to track which compressed buffer we want the size of, which is 
precisely what VideoBuffer* does for us.


> or this could also be all hidden in an video_encoder_marshall(...) 
> vfunc

I'd rather have the video encoder only care about video encoding and 
leave the decision of what to do with the compressed buffer to the 
caller, i.e. leave the marshalling to the rest of the Spice code.


> pull_compressed_buffer() could do (something like):
> 
> buffer->opaque = gst_app_sink_pull_buffer(encoder->appsink);
> buffer->free_func = gst_buffer_unref;
> if (buffer->opaque) {
> buffer->data = GST_BUFFER_DATA(video_buffer->opaque);
> buffer->size = GST_BUFFER_SIZE(video_buffer->opaque);
> buffer->free_func = gst_buffer_unref;
> return VIDEO_ENCODER_FRAME_ENCODE_DONE;
> }
> 
> and red_marshall_stream_data() would do:
> VideoBuffer buffer;
> agent->video_encoder->encode_frame(..., );
> spice_marshaller_add_ref_full(base_marshaller, buffer->data, buffer->size,
>   buffer->free_func, buffer->opaque);

I'm not sure I follow what you're getting at here.

gst_buffer_unref() and spice_marshaller_item_free_func() don't take the 
same number of parameters. The above spice_marshaller_add_ref_full() 
call would result in calling gst_buffer_unref(buffer->data, buffer->opaque).
That's why red_release_video_encoder_buffer() is needed.


[...]
> > +/* The default video buffer */
> > +GstVideoBuffer *default_buffer;
> 
> Do we really need a default video buffer rather than always allocating a
> new one? How much performance do we gain from that? Imo splitting this
> in a separate commit (or removing it) would make the whole patch much
> simpler (no need for refcounting in particular).

You're probably right that avoiding reallocations is probably overkill 
(for both the mjpeg and gstreamer encoders). After all we do allocate 
GstBuffer and GstMemory structures with every run (although my 
understanding is that GStreamer tries to cache those and that Spice does 
the same with its display_stream structures). So I'll separate that 
part.


-- 
Francois Gouget 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel