Avoiding non-essential function WiperCollectDiskMajors in WiperPartition_Open

[PengJi]

Describe the bug

It's not strictly a bug.
In file open-vm-tools/lib/wiper/wiperPosix.c there is the following function:

Bool
WiperPartition_Open(WiperPartition_List *pl,
                    Bool shrinkableOnly)
{
   MNTHANDLE fp;
   DECLARE_MNTINFO(mnt);
   Bool rc = TRUE;

   ASSERT(initDone);

   DblLnkLst_Init(&pl->link);

   /* Basically call functions to parse mounts table */
   fp = WiperOpenMountFile();
   if (fp == NULL) {
      return FALSE;
   }

   WiperCollectDiskMajors();

   ...

function WiperCollectDiskMajors is used to collect the major mumber of a disk in guest to populate knownDiskMajor, which is then used in the WiperIsDiskDevice function. WiperIsDiskDevice is called by WiperPartitionFilter, the relevant code is as follows:

static void
WiperPartitionFilter(WiperPartition *item,         // IN/OUT
                     MNTINFO *mnt,                 // IN
                     Bool shrinkableOnly)          // IN
{
   ...

   if (i == ARRAYSIZE(gKnownPartitions)) {
      comment = "Unknown filesystem. Contact VMware.";
   } else if (item->type != PARTITION_UNSUPPORTED &&
              shrinkableOnly) {
      /*
       * If the partition is supported by the wiper library, do some other
       * checks before declaring it shrinkable.
       */

      if (info->diskBacked) {
         if (Posix_Stat(MNTINFO_NAME(mnt), &s) < 0) {
            comment = "Unknown device.";
#if defined(sun) || defined(__linux__)
         } else if (!S_ISBLK(s.st_mode)) {
            comment = "Not a block device.";
#endif
         } else if (!WiperIsDiskDevice(mnt, &s)) {
            comment = "Not a disk device.";
         } else if (MNTINFO_MNT_IS_RO(mnt)) {
            comment = "Not writable.";
         }
      } else if (Posix_Access(MNTINFO_MNTPT(mnt), W_OK) != 0) {
         comment = "Mount point not writable.";
      }

      if (comment != NULL) {
         item->type = PARTITION_UNSUPPORTED;
      }
   }

   ...

}

WiperIsDiskDevice will only be called if shrinkableOnly is true, so there's a enhancement that can be made.
In function WiperPartition_Open, add following:

   if(shrinkableOnly) {
      WiperCollectDiskMajors();
   }

In function WiperSinglePartition_Open, add following:

   if(shrinkableOnly) {
      WiperCollectDiskMajors();
   }

Is that reasonable?

Reproduction steps

NULL

Expected behavior

NULL

Additional context

No response

[PaTHml]

It seems reasonable, diskInfo::GuestInfoGetDiskInfoWiper() calls WiperPartition_Open() with FALSE. I don't see knownDiskMajor being used on that codepath (shrinkableOnly=FALSE).

There is a caveat that if WiperPartitionFilter changes how it handles shrinkableOnly, or WiperIsDiskDevice is ever used outside of a shinkableOnly == TRUE codepath will have a potential inconsistency.

Some testing required...

[PaTHml]

Internal enhancement request filed.

[PaTHml]

Actually, before we can integrate this you'll need to go to the pull request process and accept the CLA.

[PengJi]

Actually, before we can integrate this you'll need to go to the pull request process and accept the CLA.

OK, I will submit a pr for this.