| Message ID | 20240804130936.1339847-1-richard.purdie@linuxfoundation.org |
|---|---|
| State | Accepted, archived |
| Commit | 03596904392d257572a905a182b92c780d636744 |
| Headers | show |
| Series | [1/2] cve_check: Use a local copy of the database during builds | expand |
On Sun, Aug 4, 2024 at 3:09 PM Richard Purdie via lists.openembedded.org <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > Rtaher than trying to use a sqlite database over NFS from DL_DIR, work from > a local copy in STAGING DIR after fetching. > I saw this patch this morning when I was nearly done with my version and waiting for the fresh build to finish... Do you plan to keep it for longer or is it for testing only? If for longer, I would likely keep old variable names for cve-update-nvd2-native.bb Did it help with the corruption issue? Kind regards, Marta
On Mon, 2024-08-05 at 17:41 +0200, Marta Rybczynska wrote: > On Sun, Aug 4, 2024 at 3:09 PM Richard Purdie via lists.openembedded.org <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > > Rtaher than trying to use a sqlite database over NFS from DL_DIR, > > work from > > a local copy in STAGING DIR after fetching. > > I saw this patch this morning when I was nearly done with my version > and waiting for the fresh build to finish... Sorry, I never saw an email saying you were writing the patch or a reply to my mail asking about it, I only wanted to try and help :( > Do you plan to keep it for longer or is it for testing only? If for > longer, I would likely keep old variable names for cve-update-nvd2- > native.bb > > Did it help with the corruption issue? It is really hard to say as it is an intermittent issue. I ended up merging it earlier on today, partly as I wanted to get it wider exposure for testing to see whether it really does help or not. It will take time to see whether it recurs. Regardless, operating on a local copy of the database rather than a shared copy in DL_DIR should help performance and avoid some sets of potential NFS issues. Which variable names did you want to keep? I kept the wider class/public names the same and only changed things within the recipe to try and ensure fewer compatibility issues. Happy to look at a different version of the patch/approach on top of mine. Cheers, Richard
On Mon, Aug 5, 2024 at 5:50 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Mon, 2024-08-05 at 17:41 +0200, Marta Rybczynska wrote: > > On Sun, Aug 4, 2024 at 3:09 PM Richard Purdie via lists.openembedded.org > <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > > > Rtaher than trying to use a sqlite database over NFS from DL_DIR, > > > work from > > > a local copy in STAGING DIR after fetching. > > > > I saw this patch this morning when I was nearly done with my version > > and waiting for the fresh build to finish... > > Sorry, I never saw an email saying you were writing the patch or a > reply to my mail asking about it, I only wanted to try and help :( > Bad communication from my side, sorry. > > > Do you plan to keep it for longer or is it for testing only? If for > > longer, I would likely keep old variable names for cve-update-nvd2- > > native.bb > > > > Did it help with the corruption issue? > > It is really hard to say as it is an intermittent issue. I ended up > merging it earlier on today, partly as I wanted to get it wider > exposure for testing to see whether it really does help or not. It will > take time to see whether it recurs. > > Regardless, operating on a local copy of the database rather than a > shared copy in DL_DIR should help performance and avoid some sets of > potential NFS issues. > > Which variable names did you want to keep? I kept the wider > class/public names the same and only changed things within the recipe > to try and ensure fewer compatibility issues. Happy to look at a > different version of the patch/approach on top of mine. > My approach was basically the same, but I kept the DL_DIR for the initial download for compatibility reasons. If it actually helps, I will send an update of variable names. Let's wait a day, what do you think? Regards, Marta
On Mon, 2024-08-05 at 18:16 +0200, Marta Rybczynska wrote: > On Mon, Aug 5, 2024 at 5:50 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Mon, 2024-08-05 at 17:41 +0200, Marta Rybczynska wrote: > > > On Sun, Aug 4, 2024 at 3:09 PM Richard Purdie via lists.openembedded.org <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > > > > > > > > > Do you plan to keep it for longer or is it for testing only? If for > > > longer, I would likely keep old variable names for cve-update-nvd2- > > > native.bb > > > > > > Did it help with the corruption issue? > > > > It is really hard to say as it is an intermittent issue. I ended up > > merging it earlier on today, partly as I wanted to get it wider > > exposure for testing to see whether it really does help or not. It will > > take time to see whether it recurs. > > > > Regardless, operating on a local copy of the database rather than a > > shared copy in DL_DIR should help performance and avoid some sets of > > potential NFS issues. > > > > Which variable names did you want to keep? I kept the wider > > class/public names the same and only changed things within the recipe > > to try and ensure fewer compatibility issues. Happy to look at a > > different version of the patch/approach on top of mine. > > > > > My approach was basically the same, but I kept the DL_DIR for the initial > download for compatibility reasons. If it actually helps, I will send an > update of variable names. Let's wait a day, what do you think? My version keeps the initial download in DL_DIR? Cheers, Richard
On Mon, Aug 5, 2024 at 6:40 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Mon, 2024-08-05 at 18:16 +0200, Marta Rybczynska wrote: > > On Mon, Aug 5, 2024 at 5:50 PM Richard Purdie < > richard.purdie@linuxfoundation.org> wrote: > > > On Mon, 2024-08-05 at 17:41 +0200, Marta Rybczynska wrote: > > > > On Sun, Aug 4, 2024 at 3:09 PM Richard Purdie via > lists.openembedded.org <richard.purdie= > linuxfoundation.org@lists.openembedded.org> wrote: > > > > > > > > > > > > Do you plan to keep it for longer or is it for testing only? If for > > > > longer, I would likely keep old variable names for cve-update-nvd2- > > > > native.bb > > > > > > > > Did it help with the corruption issue? > > > > > > It is really hard to say as it is an intermittent issue. I ended up > > > merging it earlier on today, partly as I wanted to get it wider > > > exposure for testing to see whether it really does help or not. It will > > > take time to see whether it recurs. > > > > > > Regardless, operating on a local copy of the database rather than a > > > shared copy in DL_DIR should help performance and avoid some sets of > > > potential NFS issues. > > > > > > Which variable names did you want to keep? I kept the wider > > > class/public names the same and only changed things within the recipe > > > to try and ensure fewer compatibility issues. Happy to look at a > > > different version of the patch/approach on top of mine. > > > > > > > > > My approach was basically the same, but I kept the DL_DIR for the initial > > download for compatibility reasons. If it actually helps, I will send an > > update of variable names. Let's wait a day, what do you think? > > My version keeps the initial download in DL_DIR? > I meant the variable name. I'm sending a patch, will be easier to discuss this way. Kind regards, Marta
diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass index 504310514e4..0d7c8a58354 100644 --- a/meta/classes/cve-check.bbclass +++ b/meta/classes/cve-check.bbclass @@ -31,8 +31,9 @@ CVE_PRODUCT ??= "${BPN}" CVE_VERSION ??= "${PV}" -CVE_CHECK_DB_DIR ?= "${DL_DIR}/CVE_CHECK" -CVE_CHECK_DB_FILE ?= "${CVE_CHECK_DB_DIR}/nvdcve_2-1.db" +CVE_CHECK_DB_FILENAME ?= "nvdcve_2-1.db" +CVE_CHECK_DB_DIR ?= "${STAGING_DIR}/CVE_CHECK" +CVE_CHECK_DB_FILE ?= "${CVE_CHECK_DB_DIR}/${CVE_CHECK_DB_FILENAME}" CVE_CHECK_DB_FILE_LOCK ?= "${CVE_CHECK_DB_FILE}.lock" CVE_CHECK_LOG ?= "${T}/cve.log" @@ -198,7 +199,7 @@ python do_cve_check () { } addtask cve_check before do_build -do_cve_check[depends] = "cve-update-nvd2-native:do_fetch" +do_cve_check[depends] = "cve-update-nvd2-native:do_unpack" do_cve_check[nostamp] = "1" python cve_check_cleanup () { diff --git a/meta/recipes-core/meta/cve-update-nvd2-native.bb b/meta/recipes-core/meta/cve-update-nvd2-native.bb index 1901641965a..5063f1fc3fd 100644 --- a/meta/recipes-core/meta/cve-update-nvd2-native.bb +++ b/meta/recipes-core/meta/cve-update-nvd2-native.bb @@ -8,7 +8,6 @@ INHIBIT_DEFAULT_DEPS = "1" inherit native -deltask do_unpack deltask do_patch deltask do_configure deltask do_compile @@ -35,7 +34,9 @@ CVE_DB_INCR_UPDATE_AGE_THRES ?= "10368000" # Number of attempts for each http query to nvd server before giving up CVE_DB_UPDATE_ATTEMPTS ?= "5" -CVE_DB_TEMP_FILE ?= "${CVE_CHECK_DB_DIR}/temp_nvdcve_2.db" +CVE_CHECK_DB_DLDIR_FILE ?= "${DL_DIR}/CVE_CHECK/${CVE_CHECK_DB_FILENAME}" +CVE_CHECK_DB_DLDIR_LOCK ?= "${CVE_CHECK_DB_DLDIR_FILE}.lock" +CVE_CHECK_DB_TEMP_FILE ?= "${CVE_CHECK_DB_FILE}.tmp" python () { if not bb.data.inherits_class("cve-check", d): @@ -52,9 +53,9 @@ python do_fetch() { bb.utils.export_proxies(d) - db_file = d.getVar("CVE_CHECK_DB_FILE") + db_file = d.getVar("CVE_CHECK_DB_DLDIR_FILE") db_dir = os.path.dirname(db_file) - db_tmp_file = d.getVar("CVE_DB_TEMP_FILE") + db_tmp_file = d.getVar("CVE_CHECK_DB_TEMP_FILE") cleanup_db_download(db_file, db_tmp_file) # By default let's update the whole database (since time 0) @@ -77,6 +78,7 @@ python do_fetch() { pass bb.utils.mkdirhier(db_dir) + bb.utils.mkdirhier(os.path.dirname(db_tmp_file)) if os.path.exists(db_file): shutil.copy2(db_file, db_tmp_file) @@ -89,10 +91,16 @@ python do_fetch() { os.remove(db_tmp_file) } -do_fetch[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}" +do_fetch[lockfiles] += "${CVE_CHECK_DB_DLDIR_LOCK}" do_fetch[file-checksums] = "" do_fetch[vardeps] = "" +python do_unpack() { + import shutil + shutil.copyfile(d.getVar("CVE_CHECK_DB_DLDIR_FILE"), d.getVar("CVE_CHECK_DB_FILE")) +} +do_unpack[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}" + def cleanup_db_download(db_file, db_tmp_file): """ Cleanup the download space from possible failed downloads
Rtaher than trying to use a sqlite database over NFS from DL_DIR, work from a local copy in STAGING DIR after fetching. Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- meta/classes/cve-check.bbclass | 7 ++++--- .../meta/cve-update-nvd2-native.bb | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-)