From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH for-4.5 v2 2/2] tools/[lib]xl: Fix `xl vcpu-list` when domains without vcpus exist Date: Tue, 21 Oct 2014 14:08:37 +0100 Message-ID: <54465AD5.5020209@citrix.com> References: <1413822093-1621-3-git-send-email-andrew.cooper3@citrix.com> <1413823134-2237-1-git-send-email-andrew.cooper3@citrix.com> <20141021130632.GC10234@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20141021130632.GC10234@zion.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: Wei Liu Cc: Ian Jackson , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org On 21/10/14 14:06, Wei Liu wrote: > On Mon, Oct 20, 2014 at 05:38:54PM +0100, Andrew Cooper wrote: >> On a system which looks like this: >> >> [root@st04 ~]# xl list >> Name ID Mem VCPUs State Time(s) >> Domain-0 0 752 4 r----- 46699.3 >> (null) 1 0 0 --p--- 0.0 >> (null) 2 0 0 --p--- 0.0 >> (null) 3 0 0 --p--- 0.0 >> badger 25 0 1 --p--- 0.0 >> >> `xl vcpu-list` failes as so: >> >> [root@st04 ~]# xl vcpu-list >> Name ID VCPU CPU State Time(s) CPU Affinity >> Domain-0 0 0 0 -b- 12171.0 all >> Domain-0 0 1 1 -b- 11779.6 all >> Domain-0 0 2 2 -b- 11599.0 all >> Domain-0 0 3 3 r-- 11007.0 all >> libxl: critical: libxl__calloc: libxl: FATAL ERROR: memory allocation failure (libxl__calloc, 4294935299 x 40) >> : Cannot allocate memory >> libxl: FATAL ERROR: memory allocation failure (libxl__calloc, 4294935299 x 40) >> >> The root cause of this is that, in the case that a domain has no vcpus, >> libxl_list_vcpu() attempts to calloc() with domaininfo.max_vcpu_id, which >> is uninitialised on the stack. >> >> Check domaininfo.max_vcpu_id against the new sentinel value >> XEN_INVALID_MAX_VCPU_ID (introduced in the previous patch), and return early. >> This means that it is now valid for libxl_list_vcpu() to return NULL for a >> domain which lacks any vcpus. >> >> As part of this change, remove the pointless call to libxl_get_max_cpus(), >> whose returned value is unconditionally clobbered in the for() loop. >> >> Reported-by: Euan Harris >> Signed-off-by: Andrew Cooper >> CC: Ian Campbell >> CC: Ian Jackson >> CC: Wei Liu >> >> --- >> v2: s/nr_online_cpus/max_vcpu_id/g - I accidentally sent an unrefreshed patch >> --- >> tools/libxl/libxl.c | 8 +++++++- >> tools/libxl/xl_cmdimpl.c | 4 +--- >> 2 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >> index f27b581..b7a9952 100644 >> --- a/tools/libxl/libxl.c >> +++ b/tools/libxl/libxl.c >> @@ -5246,7 +5246,13 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid, >> GC_FREE; >> return NULL; >> } >> - *nr_cpus_out = libxl_get_max_cpus(ctx); >> + >> + if (domaininfo.max_vcpu_id == XEN_INVALID_MAX_VCPU_ID) { >> + *nr_cpus_out = 0; > Do you not need to set nr_vcpus_out to 0 as well? Hmm - probably not. NULL should do. I will respin a v3 with the hypervisor and tool sides merged ~Andrew