[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/2] qdisk - hw/block/xen_disk: grant copy im
From: |
Anthony PERARD |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/2] qdisk - hw/block/xen_disk: grant copy implementation |
Date: |
Wed, 10 Aug 2016 11:29:47 +0100 |
User-agent: |
Mutt/1.6.2 (2016-07-01) |
On Tue, Aug 09, 2016 at 07:34:14PM +0200, Paulina Szubarczyk wrote:
> On 08/09/2016 06:56 PM, Anthony PERARD wrote:
> > On Tue, Aug 02, 2016 at 04:06:30PM +0200, Paulina Szubarczyk wrote:
> > > diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> > > index 640c31e..e80c61f 100644
> > > --- a/include/hw/xen/xen_common.h
> > > +++ b/include/hw/xen/xen_common.h
> > > @@ -25,6 +25,31 @@
> > > */
> > >
> > > /* Xen 4.2 through 4.6 */
> > > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 480
> > > +
> > > +struct xengnttab_grant_copy_segment {
> > > + union xengnttab_copy_ptr {
> > > + void *virt;
> > > + struct {
> > > + uint32_t ref;
> > > + uint16_t offset;
> > > + uint16_t domid;
> > > + } foreign;
> > > + } source, dest;
> > > + uint16_t len;
> > > + uint16_t flags;
> > > + int16_t status;
> > > +};
> >
> > I don't think it's a good idee to define a struct that is not going to
> > be used, and does not belong here. The typedef is OK.
>
> I added it since it is needed to know all the fields of that struct in
> ioreq_copy but if I could replace that function with stubs I will do that.
>
> >
> > In xen_disk.c, you could use "#if CONFIG_XEN_CTRL_INTERFACE_VERSION ..."
> > around free_buffers, ioreq_init_copy_buffers and ioreq_copy, and replace
> > them by stubs when Xen does not support grant copy. I think that would
> > be better.
> >
> > Also, could you try to compile again xen unstable, without your other patch?
> > Right now, it does not compile.
>
> That is true, I am sorry. I need to add the headers that are included in the
> #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 471 */
>
> #include <xenevtchn.h>
> #include <xengnttab.h>
> #include <xenforeignmemory.h>
>
> #endif
>
> May I move that part to separate #if CONFIG_XEN_CTRL_INTERFACE_VERSION >=
> 471 in that header?
You could could add your code at the end of xen_common.h instead of the
beginning, I think that would work. (At the end of xen_common.h,
xengnttab_handle should be define, either in the file or via an
include.)
> >
> > > +typedef struct xengnttab_grant_copy_segment
> > > xengnttab_grant_copy_segment_t;
> > > +
> > > +static inline int xengnttab_grant_copy(xengnttab_handle *xgt, uint32_t
> > > count,
> > > + xengnttab_grant_copy_segment_t
> > > *segs)
> > > +{
> > > + return -1;
> >
> > return -ENOSYS would be more appropriate.
> >
> >
> > Otherwise, the patch looks good.
> >
> > Thanks,
> >
> Thank you,
>
> Paulina
--
Anthony PERARD