lib: Be consistent about vm->pid

The virDomainObj struct has @pid member where the domain's
hypervisor PID is stored (e.g. QEMU/bhyve/libvirt_lxc/... PID).
However, we are not consistent when it comes to shutoff state.
Initially, because virDomainObjNew() uses g_new0() the @pid is
initialized to 0. But when domain is shut off, some functions set
it to -1 (virBhyveProcessStop, virCHProcessStop, qemuProcessStop,
..).

In other places, the @pid is tested to be 0, on some other places
it's tested for being negative and in the rest for being
positive.

To solve this inconsistency we can stick with either value, -1 or
0. I've chosen the latter as it's safer IMO. For instance if by
mistake we'd kill(vm->pid, SIGTERM) we would kill ourselves
instead of init's process group.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
This commit is contained in:
Michal Privoznik 2022-05-26 09:07:56 +02:00
parent 506210aab9
commit f344005547
10 changed files with 15 additions and 16 deletions

View File

@ -293,7 +293,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
return 0;
}
if (vm->pid <= 0) {
if (vm->pid == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PID %d for VM"),
(int)vm->pid);
@ -329,7 +329,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
bhyveProcessAutoDestroy);
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
vm->pid = -1;
vm->pid = 0;
vm->def->id = -1;
bhyveProcessStopHook(vm, VIR_HOOK_BHYVE_OP_RELEASE);
@ -344,7 +344,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
int
virBhyveProcessShutdown(virDomainObj *vm)
{
if (vm->pid <= 0) {
if (vm->pid == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PID %d for VM"),
(int)vm->pid);
@ -433,7 +433,7 @@ virBhyveProcessReconnect(virDomainObj *vm,
if (!virDomainObjIsActive(vm))
return 0;
if (!vm->pid)
if (vm->pid == 0)
return 0;
virObjectLock(vm);

View File

@ -405,7 +405,7 @@ virCHDomainGetMachineName(virDomainObj *vm)
virCHDriver *driver = priv->driver;
char *ret = NULL;
if (vm->pid > 0) {
if (vm->pid != 0) {
ret = virSystemdGetMachineNameByPID(vm->pid);
if (!ret)
virResetLastError();

View File

@ -574,7 +574,7 @@ virCHProcessStop(virCHDriver *driver G_GNUC_UNUSED,
vm->def->name);
}
vm->pid = -1;
vm->pid = 0;
vm->def->id = -1;
g_clear_pointer(&priv->machineName, g_free);

View File

@ -3059,7 +3059,7 @@ struct _virDomainObj {
virObjectLockable parent;
virCond cond;
pid_t pid;
pid_t pid; /* 0 for no PID, avoid negative values like -1 */
virDomainStateReason state;
unsigned int autostart : 1;

View File

@ -517,7 +517,7 @@ virDomainCgroupSetupCgroup(const char *prefix,
bool privileged,
char *machineName)
{
if (!vm->pid) {
if (vm->pid == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot setup cgroups until process is started"));
return -1;

View File

@ -217,7 +217,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver,
lxcProcessRemoveDomainStatus(cfg, vm);
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
vm->pid = -1;
vm->pid = 0;
vm->def->id = -1;
if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
@ -892,7 +892,7 @@ int virLXCProcessStop(virLXCDriver *driver,
_("Some processes refused to die"));
return -1;
}
} else if (vm->pid > 0) {
} else if (vm->pid != 0) {
/* If cgroup doesn't exist, just try cleaning up the
* libvirt_lxc process */
if (virProcessKillPainfully(vm->pid, true) < 0) {
@ -1033,7 +1033,7 @@ virLXCProcessReadLogOutputData(virDomainObj *vm,
bool isdead = false;
char *eol;
if (vm->pid <= 0 ||
if (vm->pid == 0 ||
(kill(vm->pid, 0) == -1 && errno == ESRCH))
isdead = true;

View File

@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driver *driver)
if (STREQ(status, "stopped")) {
virDomainObjSetState(dom, VIR_DOMAIN_SHUTOFF,
VIR_DOMAIN_SHUTOFF_UNKNOWN);
dom->pid = -1;
dom->pid = 0;
} else {
virDomainObjSetState(dom, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN);

View File

@ -10741,7 +10741,7 @@ qemuDomainGetMachineName(virDomainObj *vm)
virQEMUDriver *driver = priv->driver;
char *ret = NULL;
if (vm->pid > 0) {
if (vm->pid != 0) {
ret = virSystemdGetMachineNameByPID(vm->pid);
if (!ret)
virResetLastError();

View File

@ -8233,7 +8233,7 @@ void qemuProcessStop(virQEMUDriver *driver,
g_clear_pointer(&vm->deprecations, g_free);
vm->ndeprecations = 0;
vm->taint = 0;
vm->pid = -1;
vm->pid = 0;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
for (i = 0; i < vm->def->niothreadids; i++)
vm->def->iothreadids[i]->thread_id = 0;
@ -8963,7 +8963,7 @@ qemuProcessReconnectHelper(virDomainObj *obj,
g_autofree char *name = NULL;
/* If the VM was inactive, we don't need to reconnect */
if (!obj->pid)
if (obj->pid == 0)
return 0;
data = g_new0(struct qemuProcessReconnectData, 1);

View File

@ -54,7 +54,6 @@ prepareObjects(virQEMUDriver *driver,
if (!(vm = virDomainObjNew(driver->xmlopt)))
return -1;
vm->pid = -1;
priv = vm->privateData;
priv->chardevStdioLogd = false;
priv->rememberOwner = true;