From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Pau Monne Subject: Re: [PATCH v5 04/10] libxl: convert libxl_device_disk_add to an asyn op Date: Thu, 7 Jun 2012 18:42:20 +0200 Message-ID: <4FD0D9EC.4050700@citrix.com> References: <1338383275-21315-1-git-send-email-roger.pau@citrix.com> <1338383275-21315-5-git-send-email-roger.pau@citrix.com> <20432.47266.847506.330124@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20432.47266.847506.330124@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: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org Ian Jackson wrote: > Roger Pau Monne writes ("[PATCH v5 04/10] libxl: convert libxl_device_disk_add to an asyn op"): >> This patch converts libxl_device_disk_add to an ao operation that >> waits for device backend to reach state XenbusStateInitWait and then >> marks the operation as completed. This is not really useful now, but >> will be used by latter patches that will launch hotplug scripts after >> we reached the desired xenbus state. > > Just spotted: > >> +void libxl__initiate_device_add(libxl__egc *egc, libxl__ao_device *aodev) >> +{ > ... >> + libxl__ev_devstate_init(&aodev->ds); >> + rc = libxl__ev_devstate_wait(gc,&aodev->ds, device_backend_callback, >> + state_path, XenbusStateInitWait, >> + LIBXL_INIT_TIMEOUT * 1000); > > Another unneeded _init. Ok, I thought that you need to call libxl__ev_devstate_init before libxl__ev_devstate_wait? I just saw that _wait performs the _init itself (but without calling _init). Can't we remove _init, if it's not needed? Having and _init function makes people thing they have to use it, and the comments doesn't say anything about _wait calling _init itself. > I won't comment on any more of these I find.