From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Subject: Re: [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert} Date: Fri, 8 Apr 2016 16:51:05 +0200 Message-ID: References: <1460051129-20817-1-git-send-email-roger.pau@citrix.com> <1460051129-20817-4-git-send-email-roger.pau@citrix.com> <20160408132149.GI32191@citrix.com> <20160408142543.GO32191@citrix.com> <20160408144748.GP32191@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="0-950839033-1460127072=:18723" Return-path: Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoXkx-00042G-5G for xen-devel@lists.xenproject.org; Fri, 08 Apr 2016 14:51:31 +0000 In-Reply-To: <20160408144748.GP32191@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Wei Liu Cc: xen-devel@lists.xenproject.org, Ian Jackson , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= List-Id: xen-devel@lists.xenproject.org --0-950839033-1460127072=:18723 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT On Fri, 8 Apr 2016, Wei Liu wrote: > On Fri, Apr 08, 2016 at 04:35:22PM +0200, Roger Pau Monné wrote: > > On Fri, 8 Apr 2016, Wei Liu wrote: > > > On Fri, Apr 08, 2016 at 04:17:55PM +0200, Roger Pau Monné wrote: > > > > On Fri, 8 Apr 2016, Wei Liu wrote: > > > > > On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote: > > > > > > Signed-off-by: Roger Pau Monné > > > > > > Cc: Ian Jackson > > > > > > Cc: Wei Liu > > > > > > --- > > > > > > tools/libxl/libxl.c | 6 ++++++ > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > > > > > > index 9d785a4..232e2c1 100644 > > > > > > --- a/tools/libxl/libxl.c > > > > > > +++ b/tools/libxl/libxl.c > > > > > > @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk, > > > > > > goto out; > > > > > > } > > > > > > > > > > > > + if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) { > > > > > > + LOG(ERROR, "HVM guests without a device model cannot use cd-insert"); > > > > > > > > > > It's not specific to HVM, right? > > > > > > > > It's specific to guests with a device model (those are the only ones that > > > > can have an emulated CDROM device), and the only guests that can have a > > > > device model are HVM (although not all of them). > > > > > > > > > > Remind me again: how does PV guest CDROM work? ISTR it will also go > > > through this function? My memory is vague though... > > > > No, AFAICT PV guests use block-attach/detach in order to manage CDROMs, > > the usage of the cd-eject/insert commands is already limited to HVM > > guests, my check is only further limiting it to HVM guests with a device > > model (so it's not used for PVH guests). > > > > A little above of the check that I've added: > > > > libxl_domain_type type = libxl__domain_type(gc, domid); > > if (type == LIBXL_DOMAIN_TYPE_INVALID) { > > rc = ERROR_FAIL; > > goto out; > > } > > if (type != LIBXL_DOMAIN_TYPE_HVM) { > > LOG(ERROR, "cdrom-insert requires an HVM domain"); > > rc = ERROR_INVAL; > > goto out; > > } > > Right. Thanks for checking. I realise quibbling over an error message > seems counter-productive. > > Acked-by: Wei Liu Thanks, I don't mind changing the message to "Guests without a device model cannot use cd-insert", I just felt that adding HVM would make it clearer, but it might not. If you or the committer prefer to change the message that's fine for me (and I can send a new version if requested). Roger. --0-950839033-1460127072=:18723 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --0-950839033-1460127072=:18723--