From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH 08/11] libxl: add option for discard support to xl disk configuration Date: Thu, 6 Mar 2014 20:11:14 +0100 Message-ID: <20140306191114.GB4113@aepfle.de> References: <1394122436-1392-1-git-send-email-olaf@aepfle.de> <1394122436-1392-9-git-send-email-olaf@aepfle.de> <21272.47925.458055.605902@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <21272.47925.458055.605902@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Ian.Campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, Mar 06, Ian Jackson wrote: > Olaf Hering writes ("[PATCH 08/11] libxl: add option for discard support to xl disk configuration"): > > +This option is an advisory setting for the backend driver, depending of the > > +value, to advertise discard support (TRIM, UNMAP) to the frontend. > I think the semantics of this are unclear. Calling it an "advisory" > setting doesn't help. The documentation should explicitly explain > under what circumstances it might be honoured. > > + if (!libxl_defbool_is_default(disk->discard_enable)) > This approach is quite unusual, and arguably wrong. I think it would > be better if the default were (essentially) fixed. This would result > in discard-enable always being written here. I'm fine with writing discard-enable="1" always. > What should the backend do if the property is set but the backend > doesn't support the requested value ? The backend could advertise feature-discard anyway, so that error paths will be executed when the frontend actually sends a discard request. Right now the backend will catch this and both frontend/backend will internally disable discard. Olaf