[libvirt] [PATCH] bhyve: fix process reconnect
To reconnect to the bhyve process after deamon restart, a process VM's pid points to is checked to have proctitle equal to 'bhyve: $vmname'. However, there could be a bug in bhyve(8) which prevents it from setting proctitle, so process arguments will look like: ['/usr/sbin/bhyve', ..., 'vmname', NULL] Fall back to checking this format if proctitle doesn't match as a workaround for this bug. --- src/bhyve/bhyve_process.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 9276d7d364..78d6ff1389 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -416,18 +416,28 @@ virBhyveProcessReconnect(virDomainObjPtr vm, if (proc_argv && proc_argv[0]) { if (STREQ(expected_proctitle, proc_argv[0])) { ret = 0; - priv->mon = bhyveMonitorOpen(vm, data->driver); - if (vm->def->ngraphics == 1 && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - int vnc_port = vm->def->graphics[0]->data.vnc.port; - if (virPortAllocatorSetUsed(vnc_port) < 0) { - VIR_WARN("Failed to mark VNC port '%d' as used by '%s'", - vnc_port, vm->def->name); - } + } else { + if (STREQ(BHYVE, proc_argv[0])) { + int i = 0; + for (; proc_argv[i+1]; i++); + if ((i != 0) && (STREQ(vm->def->name, proc_argv[i]))) + ret = 0; } } } +if (ret == 0) { +priv->mon = bhyveMonitorOpen(vm, data->driver); +if (vm->def->ngraphics == 1 && +vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { +int vnc_port = vm->def->graphics[0]->data.vnc.port; +if (virPortAllocatorSetUsed(vnc_port) < 0) { +VIR_WARN("Failed to mark VNC port '%d' as used by '%s'", + vnc_port, vm->def->name); +} +} +} + cleanup: if (ret < 0) { /* If VM is reported to be in active state, but we cannot find -- 2.18.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix process reconnect
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180805155428.40866-1-bogorods...@gmail.com Subject: [libvirt] [PATCH] bhyve: fix process reconnect === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt * [new tag] patchew/cover.1534173734.git.pkre...@redhat.com -> patchew/cover.1534173734.git.pkre...@redhat.com * [new tag] patchew/e4c0c13ab8e42fa3a2571fc6f5ce303dd8fa4363.1534157377.git.mpriv...@redhat.com -> patchew/e4c0c13ab8e42fa3a2571fc6f5ce303dd8fa4363.1534157377.git.mpriv...@redhat.com Switched to a new branch 'test' f034446009 bhyve: fix process reconnect === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-4mu61k2o/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-4mu61k2o/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeerna
Re: [libvirt] [PATCH] bhyve: fix process reconnect
On 08/05/2018 05:54 PM, Roman Bogorodskiy wrote: > To reconnect to the bhyve process after deamon restart, a process > VM's pid points to is checked to have proctitle equal to 'bhyve: $vmname'. > However, there could be a bug in bhyve(8) which prevents it from > setting proctitle, so process arguments will look like: > > ['/usr/sbin/bhyve', ..., 'vmname', NULL] > > Fall back to checking this format if proctitle doesn't match as a > workaround for this bug. > --- > src/bhyve/bhyve_process.c | 26 ++ > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c > index 9276d7d364..78d6ff1389 100644 > --- a/src/bhyve/bhyve_process.c > +++ b/src/bhyve/bhyve_process.c > @@ -416,18 +416,28 @@ virBhyveProcessReconnect(virDomainObjPtr vm, > if (proc_argv && proc_argv[0]) { > if (STREQ(expected_proctitle, proc_argv[0])) { > ret = 0; > - priv->mon = bhyveMonitorOpen(vm, data->driver); > - if (vm->def->ngraphics == 1 && > - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) > { > - int vnc_port = vm->def->graphics[0]->data.vnc.port; > - if (virPortAllocatorSetUsed(vnc_port) < 0) { > - VIR_WARN("Failed to mark VNC port '%d' as used by '%s'", > - vnc_port, vm->def->name); > - } > + } else { > + if (STREQ(BHYVE, proc_argv[0])) { > + int i = 0; s/int/size_t/ > + for (; proc_argv[i+1]; i++); Ugrh. It is very easy to miss the trailing semicolon. Please move it onto a separate line. > + if ((i != 0) && (STREQ(vm->def->name, proc_argv[i]))) > + ret = 0; > } > } > } > I suggest squashing this in: diff --git i/src/bhyve/bhyve_process.c w/src/bhyve/bhyve_process.c index 78d6ff1389..47a5dc3f94 100644 --- i/src/bhyve/bhyve_process.c +++ w/src/bhyve/bhyve_process.c @@ -418,9 +418,11 @@ virBhyveProcessReconnect(virDomainObjPtr vm, ret = 0; } else { if (STREQ(BHYVE, proc_argv[0])) { - int i = 0; - for (; proc_argv[i+1]; i++); - if ((i != 0) && (STREQ(vm->def->name, proc_argv[i]))) + size_t i; + + for (i = 0; proc_argv[i+1]; i++) + ; + if ((STREQ(vm->def->name, proc_argv[i]))) ret = 0; } } ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list