On 07/10/2015 07:51 AM, Cédric Bosdonnat wrote:

The virLXCControllerAppendNBDPids function didn't properly initialize pids and npids. In case of failure it was crashing when freeing those.

The nbd device pid file doesn't appear immediately after starting qemu-nbd: adding a small loop to wait for it.

Diff to v1: * Fixed a typo in a variable name.... working with several repos leads to troubles ;) --- src/lxc/lxc_controller.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)

This should be two patches... #1 to just add the NULL for pids and 0 for npids - to fix the crash #2 to add that loop to wait for the file to appear (and of course the s/loops/loop or vice versa change ACK with the split John

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 828b8a8..78d3eee 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -533,16 +533,31 @@ static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl, const char *dev) { char *pidpath = NULL; - pid_t *pids; - size_t npids; + pid_t *pids = NULL; + size_t npids = 0; size_t i; int ret = -1; + size_t loops = 0; pid_t pid;

if (!STRPREFIX(dev, "/dev/") || virAsprintf(&pidpath, "/sys/devices/virtual/block/%s/pid", dev + 5) < 0) goto cleanup;

+ /* Wait for the pid file to appear */

How about wait for up to 1 second for the file to appear - and then strike the second comment since it'd be duplicitous

+ while (!virFileExists(pidpath)) { + /* wait for 100ms before checking again, but don't do it for ever */ + if (errno == ENOENT && loop < 10) { + usleep(100 * 1000); + loop++; + } else { + virReportSystemError(errno, + _("Cannot check NBD device %s pid"),

s/%s/'%s'

+ dev + 5); + goto cleanup; + } + } + if (virPidFileReadPath(pidpath, &pid) < 0) goto cleanup;