Address review feedback and add manpage notes

The inspect format for `.LockNumber` needed to be documented.

Signed-off-by: Matt Heon <mheon@redhat.com>
This commit is contained in:
Matt Heon
2023-06-06 09:45:32 -04:00
parent 4fda7936c5
commit 944673c883
8 changed files with 14 additions and 22 deletions

View File

@ -43,7 +43,7 @@ func runLocks() error {
if len(report.LockConflicts) > 0 { if len(report.LockConflicts) > 0 {
fmt.Printf("\nLock conflicts have been detected. Recommend immediate use of `podman system renumber` to resolve.\n\n") fmt.Printf("\nLock conflicts have been detected. Recommend immediate use of `podman system renumber` to resolve.\n\n")
} else { } else {
fmt.Printf("\nNo lock conflicts have been detected, system safe from deadlocks.\n\n") fmt.Printf("\nNo lock conflicts have been detected.\n\n")
} }
for _, lockNum := range report.LocksHeld { for _, lockNum := range report.LocksHeld {

View File

@ -43,6 +43,7 @@ Valid placeholders for the Go template are listed below:
| .IsInfra | Is this an infra container? (string: true/false) | | .IsInfra | Is this an infra container? (string: true/false) |
| .IsService | Is this a service container? (string: true/false) | | .IsService | Is this a service container? (string: true/false) |
| .KubeExitCodePropagation | Kube exit-code propagation (string) | | .KubeExitCodePropagation | Kube exit-code propagation (string) |
| .LockNumber | Number of the container's Libpod lock |
| .MountLabel | SELinux label of mount (string) | | .MountLabel | SELinux label of mount (string) |
| .Mounts | Mounts (array of strings) | | .Mounts | Mounts (array of strings) |
| .Name | Container name (string) | | .Name | Container name (string) |

View File

@ -44,6 +44,7 @@ Valid placeholders for the Go template are listed below:
| .InfraContainerID | Pod infrastructure ID | | .InfraContainerID | Pod infrastructure ID |
| .InspectPodData ... | Nested structure, for experts only | | .InspectPodData ... | Nested structure, for experts only |
| .Labels | Pod labels | | .Labels | Pod labels |
| .LockNumber | Number of the pod's Libpod lock |
| .MemoryLimit | Memory limit, bytes | | .MemoryLimit | Memory limit, bytes |
| .MemorySwap | Memory swap limit, in bytes | | .MemorySwap | Memory swap limit, in bytes |
| .Mounts | Mounts | | .Mounts | Mounts |

View File

@ -33,6 +33,7 @@ Valid placeholders for the Go template are listed below:
| .Driver | Volume driver | | .Driver | Volume driver |
| .GID | GID the volume was created with | | .GID | GID the volume was created with |
| .Labels | Label information associated with the volume | | .Labels | Label information associated with the volume |
| .LockNumber | Number of the volume's Libpod lock |
| .MountCount | Number of times the volume is mounted | | .MountCount | Number of times the volume is mounted |
| .Mountpoint | Source of volume mount point | | .Mountpoint | Source of volume mount point |
| .Name | Volume name | | .Name | Volume name |

View File

@ -42,6 +42,7 @@ Valid placeholders for the Go template are listed below:
| .GID | GID of volume | | .GID | GID of volume |
| .InspectVolumeData ... | Don't use | | .InspectVolumeData ... | Don't use |
| .Labels | Label information associated with the volume | | .Labels | Label information associated with the volume |
| .LockNumber | Number of the volume's Libpod lock |
| .MountCount | Number of times the volume is mounted | | .MountCount | Number of times the volume is mounted |
| .Mountpoint | Source of volume mount point | | .Mountpoint | Source of volume mount point |
| .Name | Volume name | | .Name | Volume name |

View File

@ -568,7 +568,7 @@ int64_t available_locks(shm_struct_t *shm) {
for (i = 0; i < shm->num_bitmaps; i++) { for (i = 0; i < shm->num_bitmaps; i++) {
// Short-circuit to catch fully-empty bitmaps quick. // Short-circuit to catch fully-empty bitmaps quick.
if (shm->locks[i].bitmap == 0) { if (shm->locks[i].bitmap == 0) {
free_locks += 32; free_locks += sizeof(bitmap_t) * 8;
continue; continue;
} }
@ -581,7 +581,7 @@ int64_t available_locks(shm_struct_t *shm) {
count++; count++;
} }
free_locks += 32 - count; free_locks += (sizeof(bitmap_t) * 8) - count;
} }
// Clear the mutex // Clear the mutex

View File

@ -1206,12 +1206,7 @@ func (r *Runtime) LockConflicts() (map[uint32][]string, []uint32, error) {
for _, ctr := range ctrs { for _, ctr := range ctrs {
lockNum := ctr.lock.ID() lockNum := ctr.lock.ID()
ctrString := fmt.Sprintf("container %s", ctr.ID()) ctrString := fmt.Sprintf("container %s", ctr.ID())
locksArr, ok := locksInUse[lockNum] locksInUse[lockNum] = append(locksInUse[lockNum], ctrString)
if ok {
locksInUse[lockNum] = append(locksArr, ctrString)
} else {
locksInUse[lockNum] = []string{ctrString}
}
} }
pods, err := r.state.AllPods() pods, err := r.state.AllPods()
@ -1221,12 +1216,7 @@ func (r *Runtime) LockConflicts() (map[uint32][]string, []uint32, error) {
for _, pod := range pods { for _, pod := range pods {
lockNum := pod.lock.ID() lockNum := pod.lock.ID()
podString := fmt.Sprintf("pod %s", pod.ID()) podString := fmt.Sprintf("pod %s", pod.ID())
locksArr, ok := locksInUse[lockNum] locksInUse[lockNum] = append(locksInUse[lockNum], podString)
if ok {
locksInUse[lockNum] = append(locksArr, podString)
} else {
locksInUse[lockNum] = []string{podString}
}
} }
volumes, err := r.state.AllVolumes() volumes, err := r.state.AllVolumes()
@ -1236,12 +1226,7 @@ func (r *Runtime) LockConflicts() (map[uint32][]string, []uint32, error) {
for _, vol := range volumes { for _, vol := range volumes {
lockNum := vol.lock.ID() lockNum := vol.lock.ID()
volString := fmt.Sprintf("volume %s", vol.Name()) volString := fmt.Sprintf("volume %s", vol.Name())
locksArr, ok := locksInUse[lockNum] locksInUse[lockNum] = append(locksInUse[lockNum], volString)
if ok {
locksInUse[lockNum] = append(locksArr, volString)
} else {
locksInUse[lockNum] = []string{volString}
}
} }
// Now go through and find any entries with >1 item associated // Now go through and find any entries with >1 item associated

View File

@ -177,7 +177,7 @@ var _ = Describe("Podman Info", func() {
Expect(session.OutputToString()).To(Equal(want)) Expect(session.OutputToString()).To(Equal(want))
}) })
It("Podman info: check lock count", func() { It("Podman info: check lock count", Serial, func() {
// This should not run on architectures and OSes that use the file locks backend. // This should not run on architectures and OSes that use the file locks backend.
// Which, for now, is Linux + RISCV and FreeBSD, neither of which are in CI - so // Which, for now, is Linux + RISCV and FreeBSD, neither of which are in CI - so
// no skips. // no skips.
@ -197,6 +197,9 @@ var _ = Describe("Podman Info", func() {
free2, err := strconv.Atoi(info2.OutputToString()) free2, err := strconv.Atoi(info2.OutputToString())
Expect(err).To(Not(HaveOccurred())) Expect(err).To(Not(HaveOccurred()))
// Effectively, we are checking that 1 lock has been taken.
// We do this by comparing the number of locks after (plus 1), to the number of locks before.
// Don't check absolute numbers because there is a decent chance of contamination, containers that were never removed properly, etc.
Expect(free1).To(Equal(free2 + 1)) Expect(free1).To(Equal(free2 + 1))
}) })
}) })