[Spice-devel] [PATCH spice-protocol v3] Add ioctls structures to qxl_windows.h

[Frediano Ziglio]

> Checkout the function " QxlDevice::Escape" in this patch:
> https://lists.freedesktop.org/archives/spice-devel/2016-August/031207.html

> This is how it is used in the driver, please note that this patch is outdated
> and needs some modifications to suit this patch of vdagent.

Yes, the two patches have different ABIs. 
Your first patch (version ago) was compatible with this one, now it's not. 
Specifically: 
- ioctl is int; 
- data structure (QXLEscapeSetCustomDisplay/QXLHead) has to follow the ioctl. Note that union does not define the packing so potentially all data structure added to the union have to be an aligned sizeof(int) or less to avoid breaking the ABI; 
- private data has to finish strictly after the specific data structure. 

Looking at your vdagent patch 
- ioctl is uint32_t (more or less the same); 
- data structure has to follow the ioctl; 
- private data has to finish after the largest data structure possible. This is a problem if you extent QXLEscape as ABI can potentially change adding an ioctl. 

Frediano 

> On Wed, Aug 17, 2016 at 10:59 AM, Frediano Ziglio < fziglio at redhat.com >
> wrote:

> > >
> 
> > > This patch defines the structures of ioctls that are used between
> 
> > > win-vdagent and the qxl-wddm-dod driver.
> 
> > >
> 
> > > Signed-off-by: Sameeh Jubran < sameeh at daynix.com >
> 
> > > ---
> 
> > > spice/qxl_windows.h | 10 +++++++++-
> 
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> > >
> 
> > > diff --git a/spice/qxl_windows.h b/spice/qxl_windows.h
> 
> > > index bc2ceff..6e9627d 100644
> 
> > > --- a/spice/qxl_windows.h
> 
> > > +++ b/spice/qxl_windows.h
> 
> > > @@ -2,7 +2,7 @@
> 
> > > #define _H_QXL_WINDOWS
> 
> > >
> 
> > > #include <spice/types.h>
> 
> > > -
> 
> > > +#include <spice/qxl_dev.h>
> 
> > > #include <spice/start-packed.h>
> 
> > >
> 
> > > enum {
> 
> > > @@ -16,6 +16,14 @@ typedef struct SPICE_ATTR_PACKED
> > > QXLEscapeSetCustomDisplay
> 
> > > {
> 
> > > uint32_t bpp;
> 
> > > } QXLEscapeSetCustomDisplay;
> 
> > >
> 
> > > +typedef struct SPICE_ATTR_PACKED QXL_ESCAPE {
> 
> > > + uint32_t ioctl;
> 
> > > + union {
> 
> > > + QXLEscapeSetCustomDisplay custom_display;
> 
> > > + QXLHead monitor_config;
> 
> > > + };
> 
> > > +} QXL_ESCAPE;
> 
> > > +
> 

> > Better QXLEscape.
> 

> > I would be wondering how you use this structure in the driver code.
> 
> > Can you post a link to a changeset?
> 

> > > #include <spice/end-packed.h>
> 
> > >
> 
> > > #endif /* _H_QXL_WINDOWS */
> 

> > Frediano
> 

> --
> Respectfully,
> Sameeh Jubran
> Linkedin
> Junior Software Engineer @ Daynix .
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160817/663150b9/attachment.html>