phosphor-software-manager: Not removing old priority on static flash layout

[mine260309]

phosphor-software-manager now supports flashing tarball of static flash layout build, and it saves the priority file in persistent storage (e.g. /var/lib/obmc/phosphor-bmc-code-mgmt).
When doing code update, the old priority file is expected to be removed.
However, due to the current code logic (ItemUpdater::freeSpace())only applying to the case that the BMC has two flash chips, it does not have a chance to remove the old priority file.

So eventually the old priority files are left on persistent storage and never get removed, e.g. on a Romulus machine:

# ls -l /var/lib/obmc/phosphor-bmc-code-mgmt
-rw-r--r--    1 root     root            21 Aug 20 07:21 0e7213e4
-rw-r--r--    1 root     root            21 Aug 16 05:40 37e01d35
-rw-r--r--    1 root     root            21 Aug  8 08:42 4565fb1e
-rw-r--r--    1 root     root            21 Aug 20 07:32 542bff4e
-rw-r--r--    1 root     root            21 Aug 20 07:35 693a4383
-rw-r--r--    1 root     root            21 Aug  8 08:18 cfeb6eb8
-rw-r--r--    1 root     root            21 Aug  8 08:51 f0f5f56c

[mine260309]

The existing code in ItemUpdater::freeSpace() makes sure:

1. It does not remove the BMC version which is functional;
2. It keeps at most ACTIVE_BMC_MAX_ALLOWED versions.

For witherspoon, it has two BMC flash chips, and ACTIVE_BMC_MAX_ALLOWED is configured to 2, so it works fine:

1. There are two BMC images, one is functional with high priority, and the other is backup with low priority;
2. When update BMC, ItemUpdater::freeSpace() it removes the backup version, and added the updated version.

For static flash layout with only one flash chip, there is always only one BMC image which is functional.
So it will not have a chance to remove it, see code below:

    for (const auto& iter : activations)
    {
        if (...)
        {
            count++;
            // Don't put the functional version on the queue since we can't
            // remove the "running" BMC version.
            if (versions.find(iter.second->versionId)->second->isFunctional())
            {
                continue;
            }
            versionsPQ.push(std::make_pair(
                iter.second->redundancyPriority.get()->priority(),
                iter.second->versionId));
        }
    }

A possible fix could be changing the logic, if ACTIVE_BMC_MAX_ALLOWED is 1, then remove functional BMC version as well.

[geissonator]

Per some recent community discussions, we're looking to have bugs assigned directly to the repo responsible.

[geissonator]

Issue moved to openbmc/phosphor-bmc-code-mgmt #2 via ZenHub