qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Cody <jcody@redhat.com>
To: Ashish Mittal <ashmit602@gmail.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, kwolf@redhat.com,
	armbru@redhat.com, berrange@redhat.com, famz@redhat.com,
	ashish.mittal@veritas.com, stefanha@gmail.com,
	Ketan.Nilangekar@veritas.com, jferlan@redhat.com,
	Buddhi.Madhav@veritas.com, Suraj.Singh@veritas.com,
	Nitin.Jerath@veritas.com, peter.maydell@linaro.org,
	venkatesha.mg@veritas.com, Rakesh.Ranjan@veritas.com,
	eblake@redhat.com, Abhijit.Dey@veritas.com
Subject: Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
Date: Tue, 11 Apr 2017 15:47:49 -0400	[thread overview]
Message-ID: <20170411194749.GA5704@localhost.localdomain> (raw)
In-Reply-To: <1491277689-24949-2-git-send-email-Ashish.Mittal@veritas.com>

On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
> Source code for the qnio library that this code loads can be downloaded from:
> https://github.com/VeritasHyperScale/libqnio.git
> 
> Sample command line using JSON syntax:
> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> -msg timestamp=on
> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> "server":{"host":"172.172.17.4","port":"9999"}}'
> 
> Sample command line using URI syntax:
> qemu-img convert -f raw -O raw -n
> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> 
> Sample command line using TLS credentials (run in secure mode):
> ./qemu-io --object
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
> 
> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>

I was testing this some with blockdev-add and blockdev-del, and this
sequence causes a segfault:

1. blockdev-add vxhs image
2. blockdev-del above image
3. blockdev-add vxhs image  <--- segfaults

Looking at it in gdb, this is an issue with libqnio.  The call to iio_fini()
is not sufficiently thorough in cleaning up resources.

In nio_client.c, qnc_ctx is never freed, because there does not
seem to be a call such as 'qnc_driver_fini' that cleans up the allocated
qnio_client_ctx.

Therefore, on the second call to iio_init, the libqnio internal variable
network_driver is NULL, because qnc_driver_init() returns NULL if it is
called when qnc_ctx is still initialized:



lib/qnio/nio_client.c:

411 int
412 iio_init(int32_t version, iio_cb_t cb)
413 {

[...]

432     apictx->network_driver = qnc_secure_driver_init(client_callback);
433     nioDbg("Created API context.\n");
434     return 0;
435 } 

[...]

779 struct channel_driver *
780 qnc_driver_init(qnio_notify client_notify)
781 {
782     if (qnc_ctx) {
783         nioDbg("Driver already initialized");
784         return NULL;
785     }
786


So two issues:

A. iio_init() should check the returned pointer, and fail if NULL

B. iio_fini() needs to clean everything up so that a new vxhs connection is
   possible. This likely means at least one new function in nio_client.c to
   clean up qnc_ctx.

-Jeff

  parent reply	other threads:[~2017-04-11 19:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
2017-04-04 15:55   ` Daniel P. Berrange
2017-04-11 19:47   ` Jeff Cody [this message]
2017-04-12  8:10     ` ashish mittal
2017-04-12 12:15       ` Jeff Cody
2017-04-19 16:27   ` Stefan Hajnoczi
2017-04-20  1:01     ` ashish mittal
2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs" Ashish Mittal
2017-04-04 18:02 ` [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Jeff Cody
2017-04-20 15:03 ` Jeff Cody
2017-04-20 16:03   ` ashish mittal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170411194749.GA5704@localhost.localdomain \
    --to=jcody@redhat.com \
    --cc=Abhijit.Dey@veritas.com \
    --cc=Buddhi.Madhav@veritas.com \
    --cc=Ketan.Nilangekar@veritas.com \
    --cc=Nitin.Jerath@veritas.com \
    --cc=Rakesh.Ranjan@veritas.com \
    --cc=Suraj.Singh@veritas.com \
    --cc=armbru@redhat.com \
    --cc=ashish.mittal@veritas.com \
    --cc=ashmit602@gmail.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=jferlan@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=venkatesha.mg@veritas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).