From 8d4b2b429d5ed15d482fb8062572118c86753a7f Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Mon, 16 Dec 2024 11:15:13 -0500 Subject: [PATCH 1/3] HPCC-33057 Allow attributes for implicit storage planes to be merged Signed-off-by: Jack Del Vecchio --- dali/base/dafdesc.cpp | 65 ++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/dali/base/dafdesc.cpp b/dali/base/dafdesc.cpp index cd903c63187..6f62b1b8541 100644 --- a/dali/base/dafdesc.cpp +++ b/dali/base/dafdesc.cpp @@ -3572,10 +3572,13 @@ void GroupInformation::createStoragePlane(IPropertyTree * storage, unsigned copy { // Check that storage plane does not already have a definition VStringBuffer xpath("planes[@name='%s']", name.str()); - if (storage->hasProp(xpath)) - return; + IPropertyTree * plane = storage->queryPropTree(xpath); + if (!plane || (copy != 0)) + { + plane = storage->addPropTree("planes"); + plane->setPropBool("@fromGroup", true); + } - IPropertyTree * plane = storage->addPropTree("planes"); StringBuffer mirrorname; const char * planeName = name; if (copy != 0) @@ -3586,37 +3589,47 @@ void GroupInformation::createStoragePlane(IPropertyTree * storage, unsigned copy //URL style drop zones don't generate a host entry, and will have a single device if (ordinality() != 0) { - if (container) - { - const char * containerName = container->name; - if (copy != 0) - containerName = mirrorname.clear().append(containerName).append("_mirror"); - //hosts will be expanded by normalizeHostGroups - plane->setProp("@hostGroup", containerName); - } - else + if (!plane->hasProp("@hostGroup")) { - //Host group has been created that matches the name of the storage plane - plane->setProp("@hostGroup", planeName); + if (container) + { + const char * containerName = container->name; + if (copy != 0) + containerName = mirrorname.clear().append(containerName).append("_mirror"); + //hosts will be expanded by normalizeHostGroups + plane->setProp("@hostGroup", containerName); + } + else + { + //Host group has been created that matches the name of the storage plane + plane->setProp("@hostGroup", planeName); + } } - if (ordinality() > 1) + if (!plane->hasProp("@numDevices")) { - plane->setPropInt("@numDevices", ordinality()); - if (dropZoneIndex == 0) - plane->setPropInt("@defaultSprayParts", ordinality()); + if (ordinality() > 1) + { + plane->setPropInt("@numDevices", ordinality()); + if (dropZoneIndex == 0) + plane->setPropInt("@defaultSprayParts", ordinality()); + } } } - if (dir.length()) - plane->setProp("@prefix", dir); - else - plane->setProp("@prefix", queryBaseDirectory(groupType, copy)); - - const char * category = (dropZoneIndex != 0) ? "lz" : "data"; - plane->setProp("@category", category); + if (!plane->hasProp("@prefix")) + { + if (dir.length()) + plane->setProp("@prefix", dir); + else + plane->setProp("@prefix", queryBaseDirectory(groupType, copy)); + } - plane->setPropBool("@fromGroup", true); + if (!plane->hasProp("@category")) + { + const char * category = (dropZoneIndex != 0) ? "lz" : "data"; + plane->setProp("@category", category); + } //MORE: If container is identical to this except for the name we could generate an information tag @alias } From 24fb144875022d6034834835dfcd18fa9d8241ab Mon Sep 17 00:00:00 2001 From: Jack Del Vecchio Date: Thu, 6 Mar 2025 09:30:04 -0500 Subject: [PATCH 2/3] Check for mirror name first --- dali/base/dafdesc.cpp | 17 ++++++++--------- system/jlib/jfile.cpp | 5 +++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dali/base/dafdesc.cpp b/dali/base/dafdesc.cpp index 6f62b1b8541..09dfff6b4a7 100644 --- a/dali/base/dafdesc.cpp +++ b/dali/base/dafdesc.cpp @@ -3570,22 +3570,21 @@ bool GroupInformation::checkIsSubset(const GroupInformation & other) void GroupInformation::createStoragePlane(IPropertyTree * storage, unsigned copy) const { + StringBuffer mirrorname; + const char * planeName = name; + if (copy != 0) + planeName = mirrorname.append(name).append("_mirror"); + // Check that storage plane does not already have a definition - VStringBuffer xpath("planes[@name='%s']", name.str()); + VStringBuffer xpath("planes[@name='%s']", planeName); IPropertyTree * plane = storage->queryPropTree(xpath); - if (!plane || (copy != 0)) + if (!plane) { plane = storage->addPropTree("planes"); + plane->setProp("@name", planeName); plane->setPropBool("@fromGroup", true); } - StringBuffer mirrorname; - const char * planeName = name; - if (copy != 0) - planeName = mirrorname.append(name).append("_mirror"); - - plane->setProp("@name", planeName); - //URL style drop zones don't generate a host entry, and will have a single device if (ordinality() != 0) { diff --git a/system/jlib/jfile.cpp b/system/jlib/jfile.cpp index 6047d5f292e..56357ae6623 100644 --- a/system/jlib/jfile.cpp +++ b/system/jlib/jfile.cpp @@ -7936,7 +7936,8 @@ MODULE_INIT(INIT_PRIORITY_STANDARD) { const IPropertyTree &plane = planesIter->query(); PlaneAttributesMapElement &element = planeAttributesMap[plane.queryProp("@name")]; - element.first = plane.queryProp("@prefix"); + const char * prefix = plane.queryProp("@prefix"); + element.first = prefix ? prefix : ""; // If prefix is empty, avoid segfault by setting it to an empty string auto &values = element.second; for (unsigned propNum=0; propNum Date: Tue, 11 Mar 2025 10:18:30 -0400 Subject: [PATCH 3/3] Fix inverted test --- system/jlib/jfile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/jlib/jfile.cpp b/system/jlib/jfile.cpp index 56357ae6623..276fa94949d 100644 --- a/system/jlib/jfile.cpp +++ b/system/jlib/jfile.cpp @@ -8046,7 +8046,7 @@ static PlaneAttributesMapElement *findPlaneElementFromPath(const char *filePath) for (auto &e: planeAttributesMap) { const char *prefix = e.second.first.c_str(); - if (isEmptyString(prefix)) // sanity check, std::string cannot be null, so check if empty + if (!isEmptyString(prefix)) // sanity check, std::string cannot be null, so check if empty { if (startsWith(filePath, prefix)) return &e.second;