diff mbox

[v5.1,6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown

Message ID 1508506702-17704-6-git-send-email-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Jackson Oct. 20, 2017, 1:38 p.m. UTC 
xc_interface_open etc. is not going to work if we have dropped
privilege, but xendevicemodel_shutdown will if everything is new
enough.

xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
provide a stub for earlier versions.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
---
v2: Add compatibility stub for Xen < 4.10.
    Fix coding style.
---
 hw/i386/xen/xen-hvm.c       | 10 ++++++++++
 include/hw/xen/xen_common.h |  7 +++++++
 2 files changed, 17 insertions(+)

Comments

Stefano Stabellini Oct. 26, 2017, 10:26 p.m. UTC | #1 
On Fri, 20 Oct 2017, Ian Jackson wrote:
> xc_interface_open etc. is not going to work if we have dropped
> privilege, but xendevicemodel_shutdown will if everything is new
> enough.
> 
> xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
> provide a stub for earlier versions.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> v2: Add compatibility stub for Xen < 4.10.
>     Fix coding style.
> ---
>  hw/i386/xen/xen-hvm.c       | 10 ++++++++++
>  include/hw/xen/xen_common.h |  7 +++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index 83420cd..25b8b14 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -1386,9 +1386,19 @@ void destroy_hvm_domain(bool reboot)
>  {
>      xc_interface *xc_handle;
>      int sts;
> +    int rc;
>  
>      unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
>  
> +    if (xen_dmod) {
> +        rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
> +        if (!rc) {
> +            return;
> +        }
> +        perror("xendevicemodel_shutdown failed");

I don't think is a good idea to print an error because this is actually
a normal condition when QEMU is build and run against an older Xen.
Users might get confused when looking at the logs.

But it would be correct to print an error if errno != ENOTTY.


> +        /* well, try the old thing then */
> +    }
> +
>      xc_handle = xc_interface_open(0, 0, 0);
>      if (xc_handle == NULL) {
>          fprintf(stderr, "Cannot acquire xenctrl handle\n");
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 8efdb8a..1d6fb57 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -108,6 +108,13 @@ static inline int xentoolcore_restrict_all(domid_t domid)
>      return -1;
>  }
>  
> +static inline int xendevicemodel_shutdown(xendevicemodel_handle *dmod,
> +                                          domid_t domid, unsigned int reason)
> +{
> +    errno = ENOTTY;
> +    return -1;
> +}
> +
>  #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
>  
>  #include <xentoolcore.h>
> -- 
> 2.1.4
>
Ian Jackson Oct. 27, 2017, 10:16 a.m. UTC | #2 
Stefano Stabellini writes ("Re: [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown"):
> On Fri, 20 Oct 2017, Ian Jackson wrote:
> > xc_interface_open etc. is not going to work if we have dropped
> > privilege, but xendevicemodel_shutdown will if everything is new
> > enough.
> > 
> > xendevicemodel_shutdown is only availabe in Xen 4.10 and later, so
> > provide a stub for earlier versions.
...
> > +    if (xen_dmod) {
> > +        rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
> > +        if (!rc) {
> > +            return;
> > +        }
> > +        perror("xendevicemodel_shutdown failed");
> 
> I don't think is a good idea to print an error because this is actually
> a normal condition when QEMU is build and run against an older Xen.
> Users might get confused when looking at the logs.

Oh.  Yes.  I wrote this before I provided the fallback stub in
xen_common.h, and therefore before I properly understood the approach
taken to fallbacks.  The fallback logic here is not correct.

> But it would be correct to print an error if errno != ENOTTY.

Indeed.

I have changed it to read like this:

    if (xen_dmod) {
        rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
        if (!rc) {
            return;
        }
        if (errno != ENOTTY /* old Xen */)
            perror("xendevicemodel_shutdown failed");
        /* well, try the old thing then */
    }

Thanks,
Ian.
diff mbox

Patch

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 83420cd..25b8b14 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1386,9 +1386,19 @@  void destroy_hvm_domain(bool reboot)
 {
     xc_interface *xc_handle;
     int sts;
+    int rc;
 
     unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
 
+    if (xen_dmod) {
+        rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
+        if (!rc) {
+            return;
+        }
+        perror("xendevicemodel_shutdown failed");
+        /* well, try the old thing then */
+    }
+
     xc_handle = xc_interface_open(0, 0, 0);
     if (xc_handle == NULL) {
         fprintf(stderr, "Cannot acquire xenctrl handle\n");
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 8efdb8a..1d6fb57 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -108,6 +108,13 @@  static inline int xentoolcore_restrict_all(domid_t domid)
     return -1;
 }
 
+static inline int xendevicemodel_shutdown(xendevicemodel_handle *dmod,
+                                          domid_t domid, unsigned int reason)
+{
+    errno = ENOTTY;
+    return -1;
+}
+
 #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 41000 */
 
 #include <xentoolcore.h>