| Message ID | 1400860669-21593-2-git-send-email-roger.pau@citrix.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 23 May 2014, Roger Pau Monne wrote: > ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the > hypervisor are translated to ENOENT. > > Also, the error code is returned in errno if the call returns -1, so > compare the error code with the value in errno instead of the value > returned by the function. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: xen-devel@lists.xenproject.org > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Anthony Perard <anthony.perard@citrix.com> > --- > Changes since v1: > - Define ENODATA to ENOENT for platforms that don't have ENODATA. > --- > xen-hvm.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/xen-hvm.c b/xen-hvm.c > index a64486c..a414105 100644 > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > start_addr >> TARGET_PAGE_BITS, npages, > bitmap); > if (rc < 0) { > - if (rc != -ENODATA) { > +#ifndef ENODATA > +#define ENODATA ENOENT > +#endif I wonder if it makes sense to have this in a more generic header, lile include/qemu/osdep.h? > + if (errno == ENODATA) { > memory_region_set_dirty(framebuffer, 0, size); > DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx > ", 0x" TARGET_FMT_plx "): %s\n", > - start_addr, start_addr + size, strerror(-rc)); > + start_addr, start_addr + size, strerror(errno)); > } > return; > } > -- > 1.7.7.5 (Apple Git-26) >
On 27/05/14 18:18, Stefano Stabellini wrote: > On Fri, 23 May 2014, Roger Pau Monne wrote: >> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the >> hypervisor are translated to ENOENT. >> >> Also, the error code is returned in errno if the call returns -1, so >> compare the error code with the value in errno instead of the value >> returned by the function. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Cc: xen-devel@lists.xenproject.org >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Cc: Anthony Perard <anthony.perard@citrix.com> >> --- >> Changes since v1: >> - Define ENODATA to ENOENT for platforms that don't have ENODATA. >> --- >> xen-hvm.c | 7 +++++-- >> 1 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/xen-hvm.c b/xen-hvm.c >> index a64486c..a414105 100644 >> --- a/xen-hvm.c >> +++ b/xen-hvm.c >> @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state, >> start_addr >> TARGET_PAGE_BITS, npages, >> bitmap); >> if (rc < 0) { >> - if (rc != -ENODATA) { >> +#ifndef ENODATA >> +#define ENODATA ENOENT >> +#endif > > I wonder if it makes sense to have this in a more generic header, lile > include/qemu/osdep.h? Well, I think this is something really specific to Xen, because FreeBSD is mapping the error returned by Xen (ENODATA), to a error code available on FreeBSD (ENOENT), see: http://xenbits.xen.org/gitweb/?p=people/royger/freebsd.git;a=patch;h=f09eeed01bc884b37e978f6ec6e3e7a86778ef4b But I don't think this is useful outside of this context, since there's no syscall that would return ENODATA on FreeBSD that instead returns ENOENT. All the paths in Qemu that check for ENODATA are Linux specific AFAICT. Roger.
On Tue, 27 May 2014, Roger Pau Monné wrote: > On 27/05/14 18:18, Stefano Stabellini wrote: > > On Fri, 23 May 2014, Roger Pau Monne wrote: > >> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the > >> hypervisor are translated to ENOENT. > >> > >> Also, the error code is returned in errno if the call returns -1, so > >> compare the error code with the value in errno instead of the value > >> returned by the function. > >> > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > >> Cc: xen-devel@lists.xenproject.org > >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> Cc: Anthony Perard <anthony.perard@citrix.com> > >> --- > >> Changes since v1: > >> - Define ENODATA to ENOENT for platforms that don't have ENODATA. > >> --- > >> xen-hvm.c | 7 +++++-- > >> 1 files changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/xen-hvm.c b/xen-hvm.c > >> index a64486c..a414105 100644 > >> --- a/xen-hvm.c > >> +++ b/xen-hvm.c > >> @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > >> start_addr >> TARGET_PAGE_BITS, npages, > >> bitmap); > >> if (rc < 0) { > >> - if (rc != -ENODATA) { > >> +#ifndef ENODATA > >> +#define ENODATA ENOENT > >> +#endif > > > > I wonder if it makes sense to have this in a more generic header, lile > > include/qemu/osdep.h? > > Well, I think this is something really specific to Xen, because FreeBSD > is mapping the error returned by Xen (ENODATA), to a error code > available on FreeBSD (ENOENT), see: > > http://xenbits.xen.org/gitweb/?p=people/royger/freebsd.git;a=patch;h=f09eeed01bc884b37e978f6ec6e3e7a86778ef4b > > But I don't think this is useful outside of this context, since there's > no syscall that would return ENODATA on FreeBSD that instead returns > ENOENT. All the paths in Qemu that check for ENODATA are Linux specific > AFAICT. Fair enough. In that case the patch is fine as it is.
diff --git a/xen-hvm.c b/xen-hvm.c index a64486c..a414105 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -499,11 +499,14 @@ static void xen_sync_dirty_bitmap(XenIOState *state, start_addr >> TARGET_PAGE_BITS, npages, bitmap); if (rc < 0) { - if (rc != -ENODATA) { +#ifndef ENODATA +#define ENODATA ENOENT +#endif + if (errno == ENODATA) { memory_region_set_dirty(framebuffer, 0, size); DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx ", 0x" TARGET_FMT_plx "): %s\n", - start_addr, start_addr + size, strerror(-rc)); + start_addr, start_addr + size, strerror(errno)); } return; }
ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the hypervisor are translated to ENOENT. Also, the error code is returned in errno if the call returns -1, so compare the error code with the value in errno instead of the value returned by the function. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: xen-devel@lists.xenproject.org Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Anthony Perard <anthony.perard@citrix.com> --- Changes since v1: - Define ENODATA to ENOENT for platforms that don't have ENODATA. --- xen-hvm.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)