diff mbox

[for-next] tools/libxc: Drop broken xc_{get, set}_hvm_param() functions

Message ID 1495453809-30108-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper May 22, 2017, 11:50 a.m. UTC
xc_{get,set}_hvm_param() are deprecated because they truncate their value
parameter in 32bit builds of libxc, and are therefore unfit for use.

As there is only a single remaining user, switch that user over to
xc_hvm_param_get() and drop these functions completely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/include/xenctrl.h |  4 ----
 tools/libxc/xc_domain.c       | 17 -----------------
 tools/xenpaging/xenpaging.c   |  7 ++++---
 3 files changed, 4 insertions(+), 24 deletions(-)

Comments

Wei Liu May 30, 2017, 10:38 a.m. UTC | #1
On Mon, May 22, 2017 at 12:50:09PM +0100, Andrew Cooper wrote:
> xc_{get,set}_hvm_param() are deprecated because they truncate their value
> parameter in 32bit builds of libxc, and are therefore unfit for use.
> 
> As there is only a single remaining user, switch that user over to
> xc_hvm_param_get() and drop these functions completely.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>
Wei Liu May 31, 2017, 10:35 a.m. UTC | #2
On Mon, May 22, 2017 at 12:50:09PM +0100, Andrew Cooper wrote:
> xc_{get,set}_hvm_param() are deprecated because they truncate their value
> parameter in 32bit builds of libxc, and are therefore unfit for use.
> 
> As there is only a single remaining user, switch that user over to
> xc_hvm_param_get() and drop these functions completely.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

This broke QEMU build because QEMU uses those.

I've reverted this patch from staging.
Andrew Cooper May 31, 2017, 11:02 a.m. UTC | #3
On 31/05/17 11:35, Wei Liu wrote:
> On Mon, May 22, 2017 at 12:50:09PM +0100, Andrew Cooper wrote:
>> xc_{get,set}_hvm_param() are deprecated because they truncate their value
>> parameter in 32bit builds of libxc, and are therefore unfit for use.
>>
>> As there is only a single remaining user, switch that user over to
>> xc_hvm_param_get() and drop these functions completely.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> This broke QEMU build because QEMU uses those.

Which qemu? trad or upstream?  I didn't encounter build errors myself.

OTOH, my commit statement still stands.  These functions are provably
unfit for purpose, and need to be gotten rid of.

~Andrew
Wei Liu May 31, 2017, 11:41 a.m. UTC | #4
On Wed, May 31, 2017 at 12:02:57PM +0100, Andrew Cooper wrote:
> On 31/05/17 11:35, Wei Liu wrote:
> > On Mon, May 22, 2017 at 12:50:09PM +0100, Andrew Cooper wrote:
> >> xc_{get,set}_hvm_param() are deprecated because they truncate their value
> >> parameter in 32bit builds of libxc, and are therefore unfit for use.
> >>
> >> As there is only a single remaining user, switch that user over to
> >> xc_hvm_param_get() and drop these functions completely.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > This broke QEMU build because QEMU uses those.
> 
> Which qemu? trad or upstream?  I didn't encounter build errors myself.
> 

qemu-trad.

https://travis-ci.org/xen-project/xen/jobs/237861522

> OTOH, my commit statement still stands.  These functions are provably
> unfit for purpose, and need to be gotten rid of.
> 

Yes. Please submit a patch to qemu-trad to switch to newer functions
first, then I can revert the revert.

Wei.

> ~Andrew
diff mbox

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1629f41..4d3f2ae 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1660,10 +1660,6 @@  void xc_clear_last_error(xc_interface *xch);
 int xc_hvm_param_set(xc_interface *handle, domid_t dom, uint32_t param, uint64_t value);
 int xc_hvm_param_get(xc_interface *handle, domid_t dom, uint32_t param, uint64_t *value);
 
-/* Deprecated: use xc_hvm_param_set/get() instead. */
-int xc_set_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long value);
-int xc_get_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long *value);
-
 /* HVM guest pass-through */
 int xc_assign_device(xc_interface *xch,
                      uint32_t domid,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 00909ad4..b619440 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1455,23 +1455,6 @@  int xc_hvm_param_get(xc_interface *handle, domid_t dom, uint32_t param, uint64_t
     return rc;
 }
 
-int xc_set_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long value)
-{
-    return xc_hvm_param_set(handle, dom, param, value);
-}
-
-int xc_get_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long *value)
-{
-    uint64_t v;
-    int ret;
-
-    ret = xc_hvm_param_get(handle, dom, param, &v);
-    if (ret < 0)
-        return ret;
-    *value = v;
-    return 0;
-}
-
 int xc_domain_setdebugging(xc_interface *xch,
                            uint32_t domid,
                            unsigned int enable)
diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index d0571ca..431a86b 100644
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -280,7 +280,8 @@  static struct xenpaging *xenpaging_init(int argc, char *argv[])
     xentoollog_logger *dbg = NULL;
     char *p;
     int rc;
-    unsigned long ring_pfn, mmap_pfn;
+    uint64_t ring_pfn;
+    unsigned long mmap_pfn;
 
     /* Allocate memory */
     paging = calloc(1, sizeof(struct xenpaging));
@@ -338,8 +339,8 @@  static struct xenpaging *xenpaging_init(int argc, char *argv[])
     }
 
     /* Map the ring page */
-    xc_get_hvm_param(xch, paging->vm_event.domain_id, 
-                        HVM_PARAM_PAGING_RING_PFN, &ring_pfn);
+    xc_hvm_param_get(xch, paging->vm_event.domain_id,
+                     HVM_PARAM_PAGING_RING_PFN, &ring_pfn);
     mmap_pfn = ring_pfn;
     paging->vm_event.ring_page = 
         xc_map_foreign_pages(xch, paging->vm_event.domain_id,