From 4e72068216b006edc69c8bafba5198051e3ed1dd Mon Sep 17 00:00:00 2001 From: David Teigland Date: Tue, 25 Jan 2022 11:35:36 -0600 Subject: [PATCH 30/54] lvmdevices: fix checks when adding entries Removes some incorrect and unnecessary checks for other entries when adding a new devices. The removed checks and corrections were mostly redundant with what is already done by device id matching. Other checking is reworked so the warnings are a bit different. --- lib/device/device_id.c | 153 +++++++++++++---------------------------- 1 file changed, 48 insertions(+), 105 deletions(-) diff --git a/lib/device/device_id.c b/lib/device/device_id.c index 625576ec6..ccc5f43a1 100644 --- a/lib/device/device_id.c +++ b/lib/device/device_id.c @@ -935,6 +935,10 @@ int device_id_add(struct cmd_context *cmd, struct device *dev, const char *pvid_ struct dev_use *du, *update_du = NULL, *du_dev, *du_pvid, *du_devname, *du_devid; struct dev_id *id; int found_id = 0; + int part = 0; + + if (!dev_get_partition_number(dev, &part)) + return_0; /* * When enable_devices_file=0 and pending_devices_file=1 we let @@ -953,10 +957,6 @@ int device_id_add(struct cmd_context *cmd, struct device *dev, const char *pvid_ */ memcpy(&pvid, pvid_arg, ID_LEN); - du_dev = get_du_for_dev(cmd, dev); - du_pvid = get_du_for_pvid(cmd, pvid); - du_devname = _get_du_for_devname(cmd, dev_name(dev)); - /* * Choose the device_id type for the device being added. * @@ -1072,6 +1072,9 @@ id_done: idtype = 0; /* + * "dev" is the device we are adding. + * "id" is the device_id it's using, set in dev->id. + * * Update the cmd->use_devices list for the new device. The * use_devices list will be used to update the devices file. * @@ -1083,23 +1086,57 @@ id_done: * those other entries to fix any incorrect info. */ + /* Is there already an entry matched to this device? */ + du_dev = get_du_for_dev(cmd, dev); + + /* Is there already an entry matched to this device's pvid? */ + du_pvid = get_du_for_pvid(cmd, pvid); + + /* Is there already an entry using this device's name? */ + du_devname = _get_du_for_devname(cmd, dev_name(dev)); + + /* Is there already an entry using the device_id for this device? */ du_devid = _get_du_for_device_id(cmd, id->idtype, id->idname); if (du_dev) - log_debug("device_id_add %s pvid %s matches du_dev %p dev %s", + log_debug("device_id_add %s pvid %s matches entry %p dev %s", dev_name(dev), pvid, du_dev, dev_name(du_dev->dev)); if (du_pvid) - log_debug("device_id_add %s pvid %s matches du_pvid %p dev %s pvid %s", + log_debug("device_id_add %s pvid %s matches entry %p dev %s with same pvid %s", dev_name(dev), pvid, du_pvid, du_pvid->dev ? dev_name(du_pvid->dev) : ".", du_pvid->pvid); if (du_devid) - log_debug("device_id_add %s pvid %s matches du_devid %p dev %s pvid %s", + log_debug("device_id_add %s pvid %s matches entry %p dev %s with same device_id %d %s", dev_name(dev), pvid, du_devid, du_devid->dev ? dev_name(du_devid->dev) : ".", - du_devid->pvid); + du_devid->idtype, du_devid->idname); if (du_devname) - log_debug("device_id_add %s pvid %s matches du_devname %p dev %s pvid %s", + log_debug("device_id_add %s pvid %s matches entry %p dev %s with same devname %s", dev_name(dev), pvid, du_devname, du_devname->dev ? dev_name(du_devname->dev) : ".", - du_devname->pvid); + du_devname->devname); + + if (du_pvid && (du_pvid->dev != dev)) + log_warn("WARNING: adding device %s with PVID %s which is already used for %s.", + dev_name(dev), pvid, du_pvid->dev ? dev_name(du_pvid->dev) : "missing device"); + + if (du_devid && (du_devid->dev != dev)) { + if (!du_devid->dev) { + log_warn("WARNING: adding device %s with idname %s which is already used for missing device.", + dev_name(dev), id->idname); + } else { + int ret1, ret2; + dev_t devt1, devt2; + /* Check if both entries are partitions of the same device. */ + ret1 = dev_get_primary_dev(cmd->dev_types, dev, &devt1); + ret2 = dev_get_primary_dev(cmd->dev_types, du_devid->dev, &devt2); + if ((ret1 == 2) && (ret2 == 2) && (devt1 == devt2)) { + log_debug("Using separate entries for partitions of same device %s part %d %s part %d.", + dev_name(dev), part, dev_name(du_devid->dev), du_devid->part); + } else { + log_warn("WARNING: adding device %s with idname %s which is already used for %s.", + dev_name(dev), id->idname, dev_name(du_devid->dev)); + } + } + } /* * If one of the existing entries (du_dev, du_pvid, du_devid, du_devname) @@ -1112,29 +1149,6 @@ id_done: dm_list_del(&update_du->list); update_matching_kind = "device"; update_matching_name = dev_name(dev); - - if (du_devid && (du_devid != du_dev)) { - log_warn("WARNING: device %s (%s) and %s (%s) have duplicate device ID.", - dev_name(dev), id->idname, - (du_pvid && du_pvid->dev) ? dev_name(du_pvid->dev) : "none", - du_pvid ? du_pvid->idname : ""); - } - - if (du_pvid && (du_pvid != du_dev)) { - log_warn("WARNING: device %s (%s) and %s (%s) have duplicate PVID %s", - dev_name(dev), id->idname, - du_pvid->dev ? dev_name(du_pvid->dev) : "none", du_pvid->idname, - pvid); - } - - if (du_devname && (du_devname != du_dev)) { - /* clear devname in another entry with our devname */ - log_warn("Devices file PVID %s clearing wrong DEVNAME %s.", - du_devname->pvid, du_devname->devname); - free(du_devname->devname); - du_devname->devname = NULL; - } - } else if (du_pvid) { /* * If the device_id of the existing entry for PVID is the same @@ -1154,11 +1168,6 @@ id_done: update_matching_kind = "PVID"; update_matching_name = pvid; } else { - log_warn("WARNING: device %s (%s) and %s (%s) have duplicate PVID %s", - dev_name(dev), id->idname, - du_pvid->dev ? dev_name(du_pvid->dev) : "none", du_pvid->idname, - pvid); - if (!cmd->current_settings.yes && yes_no_prompt("Add device with duplicate PV to devices file?") == 'n') { log_print("Device not added."); @@ -1166,21 +1175,6 @@ id_done: return 1; } } - - if (du_devid && (du_devid != du_pvid)) { - /* warn about another entry using the same device_id */ - log_warn("WARNING: duplicate device_id %s for PVIDs %s %s", - du_devid->idname, du_devid->pvid, du_pvid->pvid); - } - - if (du_devname && (du_devname != du_pvid)) { - /* clear devname in another entry with our devname */ - log_warn("Devices file PVID %s clearing wrong DEVNAME %s.", - du_devname->pvid, du_devname->devname); - free(du_devname->devname); - du_devname->devname = NULL; - } - } else if (du_devid) { /* * Do we create a new du or update the existing du? @@ -1195,64 +1189,13 @@ id_done: * the same device_id (create a new du for dev.) * If not, then update the existing du_devid. */ - - if (du_devid->dev != dev) - check_idname = device_id_system_read(cmd, du_devid->dev, id->idtype); - - if (check_idname && !strcmp(check_idname, id->idname)) { - int ret1, ret2; - dev_t devt1, devt2; - - /* - * two different devices have the same device_id, - * create a new du for the device being added - */ - - /* dev_is_partitioned() the dev open to read it. */ - if (!label_scan_open(du_devid->dev)) - log_warn("Cannot open %s", dev_name(du_devid->dev)); - - if (dev_is_partitioned(cmd, du_devid->dev)) { - /* Check if existing entry is whole device and new entry is a partition of it. */ - ret1 = dev_get_primary_dev(cmd->dev_types, dev, &devt1); - if ((ret1 == 2) && (devt1 == du_devid->dev->dev)) - log_warn("Remove partitioned device %s from devices file.", dev_name(du_devid->dev)); - } else { - /* Check if both entries are partitions of the same device. */ - ret1 = dev_get_primary_dev(cmd->dev_types, dev, &devt1); - ret2 = dev_get_primary_dev(cmd->dev_types, du_devid->dev, &devt2); - - if ((ret1 == 2) && (ret2 == 2) && (devt1 == devt2)) { - log_warn("Partitions %s %s have same device_id %s", - dev_name(dev), dev_name(du_devid->dev), id->idname); - } else { - log_warn("Duplicate device_id %s %s for %s and %s", - idtype_to_str(id->idtype), check_idname, - dev_name(dev), dev_name(du_devid->dev)); - } - } - } else { + if (du_devid->dev == dev) { /* update the existing entry with matching devid */ update_du = du_devid; dm_list_del(&update_du->list); update_matching_kind = "device_id"; update_matching_name = id->idname; } - - if (du_devname && (du_devname != du_devid)) { - /* clear devname in another entry with our devname */ - log_warn("Devices file PVID %s clearing wrong DEVNAME %s", - du_devname->pvid, du_devname->devname); - free(du_devname->devname); - du_devname->devname = NULL; - } - - } else if (du_devname) { - /* clear devname in another entry with our devname */ - log_warn("Devices file PVID %s clearing wrong DEVNAME %s", - du_devname->pvid, du_devname->devname); - free(du_devname->devname); - du_devname->devname = NULL; } free((void *)check_idname); -- 2.34.3