From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v3 2/7] libxl: Add to libxl__domain_type a new return value (LIBXL_DOMAIN_TYPE_NOTFOUND) Date: Tue, 24 Mar 2015 11:47:45 -0400 Message-ID: <20150324154745.GE14418@l.oracle.com> References: <1427134861-18881-1-git-send-email-konrad.wilk@oracle.com> <1427134861-18881-3-git-send-email-konrad.wilk@oracle.com> <1427210130.21742.434.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YaR3a-0008AC-5t for xen-devel@lists.xenproject.org; Tue, 24 Mar 2015 15:47:54 +0000 Content-Disposition: inline In-Reply-To: <1427210130.21742.434.camel@citrix.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 Campbell Cc: Wei Liu , Ian Jackson , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Tue, Mar 24, 2015 at 03:15:30PM +0000, Ian Campbell wrote: > On Mon, 2015-03-23 at 14:20 -0400, Konrad Rzeszutek Wilk wrote: > > So that the callers can distinguish between an error and > > an domain not found. The exposed API calls that are effected > > by this are: libxl_domain_[remus_start,suspend,unpause,cdrom_insert, > > set_vcpuonline] > > > > We add an helper function to deal with the two types of errors: > > libxl_domain_type2err. However for libxl_[pci,dom].c we just add > > the extra check for LIBXL_DOMAIN_TYPE_NOTFOUND for simplicity > > reasons. > > > > Suggested-by: Ian Campbell > > Did I? You suggested that 'libxl_set_vcpuonline' deal with all of the various issues instead of having them in the 'vcpusset'. This would bolting in libxl_set_vcpuonline' the proper error code support - which this patch does. > > Anyway, I'm not terribly keen on the approach taken here, sorry, in > particular extending libxl_domain_type with a subset of the libxl_error > codes and the shenanigans which are needed to convert between them. :-) > > Off hand I can think of 3 possible alternative solutions: > > Arrange that the -ve error values in libxl_domain_type directly > correspond to an appropriate libxl_error code, allowing code to do > if (type < 0) return type; > if (type < 0) { rc = type ; goto out } > type stuff. > > Change the prototype of libxl__domain_type to always return a libxl rc > and on success return the type via a pointer argument. > > Change the return type of libxl_domain_type to an int and return either > a LIBXL_DOMAIN_TYPE_FOO or an ERROR_*. > > They all have downsides (I don't much like the implicit cast between > domain type and error code, changing the prototype will probably be > disruptive, changing the return type means gcc can't catch missing > switch statements for us). > > Perhaps you or my fellow maintainers have a better idea or a preference > for one of the above? Drop the patch altogether and just leave the imprecise errors that libxl_set_vcpuonline can return? It is OK if it just returns LIBXL_DOMAIN_TYPE_INVALID - as the message about what went wrong is most important. With patches: "libxl/vcpu-set - allow to decrease vcpu count on overcommitted guests (v3)" and "libxl: In libxl_set_vcpuonline check for maximum number of VCPUs against the cpumap" we print now the error messages when the user mistakenly: 1) picks the wrong domain id 2) picks the wrong cpu count (too many) So this patch can be ignored. > > Ian. >