os/: modify getattrs to clear attrs out param before populating

Passing in a non-empty map would otherwise exhibit quite unexpected
behavior.  For the bufferptr overload, any preexisting entries would
not be overwritten due to how std::map::emplace behaves.  For the
bufferlist overload, it would result in appending to any pre-existing
entries.

The prior commit cleans up one such inadvertent caller which resulted
in the below bug.

Fixes: https://tracker.ceph.com/issues/65185
Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit 915f92ba9dc8e192bdcebcb92e7d06a93a401f5d)

diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
index 302be387fae6e62a7f303e9ff06f530d9d26ec35..4889b715267998c0b93cb65cdec69b58a294d2ab 100644 (file)
--- a/src/os/ObjectStore.h
+++ b/src/os/ObjectStore.h
@@ -611,7 +611,7 @@ public:
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   virtual int getattrs(CollectionHandle &c, const ghobject_t& oid,
@@ -622,13 +622,14 @@ public:
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   int getattrs(CollectionHandle &c, const ghobject_t& oid,
               std::map<std::string,ceph::buffer::list,std::less<>>& aset) {
     std::map<std::string,ceph::buffer::ptr,std::less<>> bmap;
     int r = getattrs(c, oid, bmap);
+    aset.clear();
     for (auto i = bmap.begin(); i != bmap.end(); ++i) {
       aset[i->first].append(i->second);
     }

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 7d02b1551e0f16d41a3a8b3b08e1affadf27426d..dda2b0b1e63a74d9c7895d0f7b6233dd5f252c73 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -12426,6 +12426,7 @@ int BlueStore::getattrs(
       r = -ENOENT;
       goto out;
     }
+    aset.clear();
     for (auto& i : o->onode.attrs) {
       aset.emplace(i.first.c_str(), i.second);
     }