From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: Re: [libvirt] [PATCH 3/5] libxl: Move virDomainXMLOptionNew into libxlCreateXMLConf Date: Mon, 2 Jun 2014 10:33:30 +0100 Message-ID: <20140602093330.GC28039@redhat.com> References: <1401470645-19869-1-git-send-email-berrange@redhat.com> <1401470645-19869-4-git-send-email-berrange@redhat.com> <53890918.9070405@suse.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <53890918.9070405@suse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com To: Jim Fehlig Cc: libvir-list@redhat.com, xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Fri, May 30, 2014 at 04:41:28PM -0600, Jim Fehlig wrote: > Daniel P. Berrange wrote: > > To allow the test suite to creat the XML option object, > > move the virDomainXMLOptionNew call into a libxlCreateXMLConf > > method. > > > > Signed-off-by: Daniel P. Berrange > > --- > > src/libxl/libxl_conf.c | 7 +++++++ > > src/libxl/libxl_conf.h | 2 ++ > > src/libxl/libxl_domain.c | 4 ++-- > > src/libxl/libxl_driver.c | 4 +--- > > 4 files changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > > index e00a3fb..00ff14f 100644 > > --- a/src/libxl/libxl_domain.c > > +++ b/src/libxl/libxl_domain.c > > @@ -1100,6 +1100,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, > > #endif > > virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; > > > > + libxl_domain_config_init(&d_config); > > + > > if (libxlDomainObjPrivateInitCtx(vm) < 0) > > return ret; > > > > @@ -1149,8 +1151,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, > > VIR_FREE(managed_save_path); > > } > > > > - libxl_domain_config_init(&d_config); > > - > > if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def, > > priv->ctx, &d_config) < 0) > > goto endjob; > > > > Are these two hunks fixing a bug you found? :-) Hmm, yes, I should have done those as a separate patch. The 'd_config' variable is stack allocated so has undefined initial state. 'libxl_domain_config_init' is basically doing a memset(0,...) on it to give it predictable value. So if we call that late in the function, there's a chance that a 'goto endjob' call will jump to the place where we call libxl_domain_config_dispose(), which will then access uninitialized memory, will unpredictably bad results. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|